From c2983daa13f85d139797eb3ca17f81c25e6997b7 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Thu, 8 Mar 2018 21:35:37 -0500 Subject: [PATCH] Fix findbugs violations in md-sal - part 3 - sal-binding-broker - sal-connector-api - sal-dummy-distributed-datastore - messagebus-impl - mdsal-trace-dom-impl - May expose internal representation by returning reference to mutable object - May expose internal representation by incorporating reference to mutable object - Private method is never called - Non-transient non-serializable instance field in serializable class - Class is Serializable, but doesn't define serialVersionUID - Parameter must be non-null but is marked as nullable - Unread field - Consider returning a zero length array rather than null - Useless object created - Method ignores return value - Incorrect lazy initialization of static field - Should be a static inner class Change-Id: Ia8847db80bca98c6f7ff7aae267efc408a5dd8fd Signed-off-by: Tom Pantelis --- .../impl/CloseTrackedRegistry.java | 3 +++ .../closetracker/impl/CloseTrackedTrait.java | 2 ++ .../md/sal/trace/dom/impl/TracingBroker.java | 2 +- .../dom/impl/TracingReadOnlyTransaction.java | 4 +--- .../dom/impl/TracingTransactionChain.java | 2 +- .../messagebus/app/impl/EventSourceTopic.java | 3 ++- ...iumNotificationProviderServiceAdapter.java | 2 +- .../impl/AbstractForwardedDataBroker.java | 13 ++++------- .../md/sal/binding/impl/FutureSchema.java | 3 ++- .../binding/codegen/impl/SingletonHolder.java | 2 +- .../binding/impl/RootBindingAwareBroker.java | 2 +- .../connector/api/BindingAwareRpcRouter.java | 3 +++ .../api/BindingAwareZeroMqRpcRouter.java | 6 ----- .../dummy/datastore/DummyShard.java | 21 +---------------- .../dummy/datastore/DummyShardManager.java | 23 +------------------ 15 files changed, 24 insertions(+), 67 deletions(-) diff --git a/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/closetracker/impl/CloseTrackedRegistry.java b/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/closetracker/impl/CloseTrackedRegistry.java index 4e60839630..78a09f1ce7 100644 --- a/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/closetracker/impl/CloseTrackedRegistry.java +++ b/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/closetracker/impl/CloseTrackedRegistry.java @@ -10,6 +10,7 @@ package org.opendaylight.controller.md.sal.trace.closetracker.impl; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -87,6 +88,8 @@ public class CloseTrackedRegistry> { * debugContextEnabled is false), and value is the number of open * instances created at that place in the code. */ + // For some reason, FB sees 'map' as useless but it clearly isn't. + @SuppressFBWarnings("UC_USELESS_OBJECT") public Set> getAllUnique() { Map, Long> map = new HashMap<>(); Set> copyOfTracked = new HashSet<>(tracked); diff --git a/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/closetracker/impl/CloseTrackedTrait.java b/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/closetracker/impl/CloseTrackedTrait.java index 4fe1e12ec8..30e81c42df 100644 --- a/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/closetracker/impl/CloseTrackedTrait.java +++ b/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/closetracker/impl/CloseTrackedTrait.java @@ -7,6 +7,7 @@ */ package org.opendaylight.controller.md.sal.trace.closetracker.impl; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Objects; import javax.annotation.Nullable; @@ -45,6 +46,7 @@ public class CloseTrackedTrait> implements CloseTracke @Override @Nullable + @SuppressFBWarnings("PZLA_PREFER_ZERO_LENGTH_ARRAYS") public StackTraceElement[] getAllocationContextStackTrace() { return allocationContext != null ? allocationContext.getStackTrace() : null; } diff --git a/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/dom/impl/TracingBroker.java b/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/dom/impl/TracingBroker.java index d6b1039631..02e65e7754 100644 --- a/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/dom/impl/TracingBroker.java +++ b/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/dom/impl/TracingBroker.java @@ -335,7 +335,7 @@ public class TracingBroker implements TracingDOMDataBroker { @Override public DOMDataReadOnlyTransaction newReadOnlyTransaction() { - return new TracingReadOnlyTransaction(delegate.newReadOnlyTransaction(), this, readOnlyTransactionsRegistry); + return new TracingReadOnlyTransaction(delegate.newReadOnlyTransaction(), readOnlyTransactionsRegistry); } @Nonnull diff --git a/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/dom/impl/TracingReadOnlyTransaction.java b/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/dom/impl/TracingReadOnlyTransaction.java index e275880fc2..56e8d90680 100644 --- a/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/dom/impl/TracingReadOnlyTransaction.java +++ b/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/dom/impl/TracingReadOnlyTransaction.java @@ -22,13 +22,11 @@ class TracingReadOnlyTransaction implements DOMDataReadOnlyTransaction { private final DOMDataReadOnlyTransaction delegate; - private final TracingBroker tracingBroker; - TracingReadOnlyTransaction(DOMDataReadOnlyTransaction delegate, TracingBroker tracingBroker, + TracingReadOnlyTransaction(DOMDataReadOnlyTransaction delegate, CloseTrackedRegistry readOnlyTransactionsRegistry) { super(readOnlyTransactionsRegistry); this.delegate = delegate; - this.tracingBroker = tracingBroker; } @Override diff --git a/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/dom/impl/TracingTransactionChain.java b/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/dom/impl/TracingTransactionChain.java index b6f2fe0f9b..2e5070f551 100644 --- a/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/dom/impl/TracingTransactionChain.java +++ b/opendaylight/md-sal/mdsal-trace/dom-impl/src/main/java/org/opendaylight/controller/md/sal/trace/dom/impl/TracingTransactionChain.java @@ -40,7 +40,7 @@ class TracingTransactionChain extends AbstractCloseTracked, Aut Futures.addCallback(future, new FutureCallback>() { @Override - public void onSuccess(final Optional data) { + public void onSuccess(@Nonnull final Optional data) { if (data.isPresent()) { final List nodes = data.get().getNode(); if (nodes != null) { diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/compat/HeliumNotificationProviderServiceAdapter.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/compat/HeliumNotificationProviderServiceAdapter.java index a65d490742..149c2c2dab 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/compat/HeliumNotificationProviderServiceAdapter.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/compat/HeliumNotificationProviderServiceAdapter.java @@ -17,7 +17,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class HeliumNotificationProviderServiceAdapter extends HeliumNotificationServiceAdapter - implements NotificationProviderService, AutoCloseable { + implements NotificationProviderService { private static final Logger LOG = LoggerFactory.getLogger(HeliumNotificationProviderServiceAdapter.class); private final NotificationPublishService notificationPublishService; diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedDataBroker.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedDataBroker.java index da9cb2ee9a..aff4a2ba09 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedDataBroker.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedDataBroker.java @@ -71,8 +71,7 @@ public abstract class AbstractForwardedDataBroker implements Delegator domRegistration = @@ -130,16 +129,12 @@ public abstract class AbstractForwardedDataBroker implements Delegator path; - private final DataChangeScope triggeringScope; - TranslatingDataChangeInvoker(final LogicalDatastoreType store, final InstanceIdentifier path, - final DataChangeListener bindingDataChangeListener, final DataChangeScope triggeringScope) { - this.store = store; + TranslatingDataChangeInvoker(final InstanceIdentifier path, + final DataChangeListener bindingDataChangeListener) { this.path = path; this.bindingDataChangeListener = bindingDataChangeListener; - this.triggeringScope = triggeringScope; } @Override @@ -161,7 +156,7 @@ public abstract class AbstractForwardedDataBroker implements Delegator path, final DataChangeListener bindingDataChangeListener, final DataChangeScope triggeringScope) { - super(store, path, bindingDataChangeListener, triggeringScope); + super(path, bindingDataChangeListener); } } diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/FutureSchema.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/FutureSchema.java index 0c378390e5..5ee1f47a28 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/FutureSchema.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/FutureSchema.java @@ -18,6 +18,7 @@ import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import javax.annotation.Nonnull; import javax.annotation.concurrent.GuardedBy; import org.opendaylight.mdsal.binding.generator.util.BindingRuntimeContext; import org.opendaylight.yangtools.yang.binding.Augmentation; @@ -90,7 +91,7 @@ class FutureSchema implements AutoCloseable { boolean waitForSchema(final QNameModule module) { return addPostponedOpAndWait(new FutureSchemaPredicate() { @Override - public boolean apply(final BindingRuntimeContext input) { + public boolean apply(@Nonnull final BindingRuntimeContext input) { return input.getSchemaContext().findModule(module).isPresent(); } }); diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/SingletonHolder.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/SingletonHolder.java index 6098684d7c..13312b51b3 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/SingletonHolder.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/SingletonHolder.java @@ -131,7 +131,7 @@ public final class SingletonHolder { return COMMIT_EXECUTOR; } - public static ExecutorService getDefaultChangeEventExecutor() { + public static synchronized ExecutorService getDefaultChangeEventExecutor() { if (CHANGE_EVENT_EXECUTOR == null) { final ThreadFactory factory = new ThreadFactoryBuilder().setDaemon(true) .setNameFormat("md-sal-binding-change-%d").build(); diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/RootBindingAwareBroker.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/RootBindingAwareBroker.java index f5f2b76f94..950384eb8e 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/RootBindingAwareBroker.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/RootBindingAwareBroker.java @@ -188,7 +188,7 @@ public class RootBindingAwareBroker implements Mutable, Identifiable, Bi return getRoot().registerRouteChangeListener(listener); } - public class RootSalInstance extends + public static class RootSalInstance extends AbstractBindingSalProviderInstance { public RootSalInstance(final RpcProviderRegistry rpcRegistry, diff --git a/opendaylight/md-sal/sal-connector-api/src/main/java/org/opendaylight/controller/sal/connector/api/BindingAwareRpcRouter.java b/opendaylight/md-sal/sal-connector-api/src/main/java/org/opendaylight/controller/sal/connector/api/BindingAwareRpcRouter.java index db10b08ff4..22366f8f85 100644 --- a/opendaylight/md-sal/sal-connector-api/src/main/java/org/opendaylight/controller/sal/connector/api/BindingAwareRpcRouter.java +++ b/opendaylight/md-sal/sal-connector-api/src/main/java/org/opendaylight/controller/sal/connector/api/BindingAwareRpcRouter.java @@ -7,6 +7,7 @@ */ package org.opendaylight.controller.sal.connector.api; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.concurrent.Future; import org.opendaylight.yangtools.concepts.Immutable; @@ -16,6 +17,7 @@ public interface BindingAwareRpcRouter extends RpcRouter> sendRpc( RpcRequest input); + @SuppressFBWarnings("EI_EXPOSE_REP2") class BindingAwareRequest implements RpcRequest, Immutable { private final BindingAwareRouteIdentifier routingInformation; @@ -32,6 +34,7 @@ public interface BindingAwareRpcRouter extends RpcRouter> receivedRequest(RpcRequest input) { - - return mdSalRouter.sendRpc(input); - } - } diff --git a/opendaylight/md-sal/sal-dummy-distributed-datastore/src/main/java/org/opendaylight/controller/dummy/datastore/DummyShard.java b/opendaylight/md-sal/sal-dummy-distributed-datastore/src/main/java/org/opendaylight/controller/dummy/datastore/DummyShard.java index db9515b43e..c5a3a032d4 100644 --- a/opendaylight/md-sal/sal-dummy-distributed-datastore/src/main/java/org/opendaylight/controller/dummy/datastore/DummyShard.java +++ b/opendaylight/md-sal/sal-dummy-distributed-datastore/src/main/java/org/opendaylight/controller/dummy/datastore/DummyShard.java @@ -10,7 +10,6 @@ package org.opendaylight.controller.dummy.datastore; import akka.actor.Props; import akka.actor.UntypedActor; -import akka.japi.Creator; import com.google.common.base.Stopwatch; import java.util.concurrent.TimeUnit; import org.opendaylight.controller.cluster.datastore.DataStoreVersions; @@ -110,24 +109,6 @@ public class DummyShard extends UntypedActor { } public static Props props(Configuration configuration, final String followerId) { - - return Props.create(new DummyShardCreator(configuration, followerId)); - } - - private static class DummyShardCreator implements Creator { - - private static final long serialVersionUID = 1L; - private final Configuration configuration; - private final String followerId; - - DummyShardCreator(Configuration configuration, String followerId) { - this.configuration = configuration; - this.followerId = followerId; - } - - @Override - public DummyShard create() throws Exception { - return new DummyShard(configuration, followerId); - } + return Props.create(DummyShard.class, configuration, followerId); } } diff --git a/opendaylight/md-sal/sal-dummy-distributed-datastore/src/main/java/org/opendaylight/controller/dummy/datastore/DummyShardManager.java b/opendaylight/md-sal/sal-dummy-distributed-datastore/src/main/java/org/opendaylight/controller/dummy/datastore/DummyShardManager.java index abc73db9ed..93d5c67bbc 100644 --- a/opendaylight/md-sal/sal-dummy-distributed-datastore/src/main/java/org/opendaylight/controller/dummy/datastore/DummyShardManager.java +++ b/opendaylight/md-sal/sal-dummy-distributed-datastore/src/main/java/org/opendaylight/controller/dummy/datastore/DummyShardManager.java @@ -11,7 +11,6 @@ package org.opendaylight.controller.dummy.datastore; import akka.actor.ActorContext; import akka.actor.Props; import akka.actor.UntypedActor; -import akka.japi.Creator; public class DummyShardManager extends UntypedActor { public DummyShardManager(Configuration configuration, String memberName, String[] shardNames, @@ -25,27 +24,7 @@ public class DummyShardManager extends UntypedActor { } public static Props props(Configuration configuration, String memberName, String[] shardNames, String type) { - return Props.create(new DummyShardManagerCreator(configuration, memberName, shardNames, type)); - } - - private static class DummyShardManagerCreator implements Creator { - - private final Configuration configuration; - private final String memberName; - private final String[] shardNames; - private final String type; - - DummyShardManagerCreator(Configuration configuration, String memberName, String[] shardNames, String type) { - this.configuration = configuration; - this.memberName = memberName; - this.shardNames = shardNames; - this.type = type; - } - - @Override - public DummyShardManager create() throws Exception { - return new DummyShardManager(configuration, memberName, shardNames, type); - } + return Props.create(DummyShardManager.class, configuration, memberName, shardNames, type); } private static class DummyShardsCreator { -- 2.36.6