From e2607370f5ac443a5a2f1f00f693f82a0b57161d Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Wed, 7 Mar 2018 21:54:09 -0500 Subject: [PATCH] Fix findbugs violations in md-sal - part 1 - sal-common-util - sal-common-impl - sal-dom-api - sal-dom-spi - sal-binding-api - sal-inmemory-datastore Violations: - Non-transient non-serializable instance field in serializable class - Field isn't final but should be - Unchecked/unconfirmed cast - Class names shouldn't shadow simple name of implemented interface - Redundant nullcheck of value known to be non-null - Field not initialized in constructor but dereferenced without null check - Equals checks for incompatible operand - Method ignores return value Change-Id: I57ceba7dae12114eba962c01aea259004f4a2983 Signed-off-by: Tom Pantelis --- .../sal/binding/api/AbstractBrokerAwareActivator.java | 4 +++- .../impl/util/compat/DataNormalizationOperation.java | 4 ++++ .../md/sal/common/util/jmx/AbstractMXBean.java | 2 +- .../opendaylight/controller/sal/common/util/Rpcs.java | 4 +--- .../md/sal/dom/api/DOMDataTreeCommitCohortRegistry.java | 3 +++ .../controller/md/sal/dom/api/DOMRpcIdentifier.java | 2 +- .../controller/sal/core/api/AbstractConsumer.java | 9 +++++---- .../controller/sal/core/api/AbstractProvider.java | 8 ++++++-- .../controller/md/sal/dom/spi/DefaultDOMRpcResult.java | 9 +++++++++ .../sal/dom/store/impl/DOMImmutableDataChangeEvent.java | 3 +-- .../md/sal/dom/store/impl/InMemoryDOMDataStore.java | 4 +--- .../store/impl/InMemoryDOMStoreTreeChangePublisher.java | 5 +---- .../md/sal/dom/store/impl/tree/ListenerNode.java | 6 +++++- 13 files changed, 41 insertions(+), 22 deletions(-) diff --git a/opendaylight/md-sal/sal-binding-api/src/main/java/org/opendaylight/controller/sal/binding/api/AbstractBrokerAwareActivator.java b/opendaylight/md-sal/sal-binding-api/src/main/java/org/opendaylight/controller/sal/binding/api/AbstractBrokerAwareActivator.java index 83af462af3..7c29846621 100644 --- a/opendaylight/md-sal/sal-binding-api/src/main/java/org/opendaylight/controller/sal/binding/api/AbstractBrokerAwareActivator.java +++ b/opendaylight/md-sal/sal-binding-api/src/main/java/org/opendaylight/controller/sal/binding/api/AbstractBrokerAwareActivator.java @@ -60,7 +60,9 @@ public abstract class AbstractBrokerAwareActivator implements BundleActivator { @Override public final void stop(BundleContext bundleContext) throws Exception { - tracker.close(); + if (tracker != null) { + tracker.close(); + } stopImpl(bundleContext); } diff --git a/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationOperation.java b/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationOperation.java index 4174fac41c..e86c1eb640 100644 --- a/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationOperation.java +++ b/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationOperation.java @@ -11,6 +11,7 @@ import com.google.common.base.Optional; import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -221,6 +222,7 @@ public abstract class DataNormalizationOperation impleme } @Override + @SuppressFBWarnings("BC_UNCONFIRMED_CAST") public NormalizedNode createDefault(final PathArgument currentArg) { final DataContainerNodeAttrBuilder builder = Builders .mapEntryBuilder().withNodeIdentifier((NodeIdentifierWithPredicates) currentArg); @@ -249,6 +251,7 @@ public abstract class DataNormalizationOperation impleme } @Override + @SuppressFBWarnings("BC_UNCONFIRMED_CAST") public NormalizedNode createDefault(final PathArgument currentArg) { return Builders.unkeyedListEntryBuilder().withNodeIdentifier((NodeIdentifier) currentArg).build(); } @@ -262,6 +265,7 @@ public abstract class DataNormalizationOperation impleme } @Override + @SuppressFBWarnings("BC_UNCONFIRMED_CAST") public NormalizedNode createDefault(final PathArgument currentArg) { return Builders.containerBuilder().withNodeIdentifier((NodeIdentifier) currentArg).build(); } diff --git a/opendaylight/md-sal/sal-common-util/src/main/java/org/opendaylight/controller/md/sal/common/util/jmx/AbstractMXBean.java b/opendaylight/md-sal/sal-common-util/src/main/java/org/opendaylight/controller/md/sal/common/util/jmx/AbstractMXBean.java index d60623aba0..3cf6d9fea4 100644 --- a/opendaylight/md-sal/sal-common-util/src/main/java/org/opendaylight/controller/md/sal/common/util/jmx/AbstractMXBean.java +++ b/opendaylight/md-sal/sal-common-util/src/main/java/org/opendaylight/controller/md/sal/common/util/jmx/AbstractMXBean.java @@ -38,7 +38,7 @@ public abstract class AbstractMXBean { private static final Logger LOG = LoggerFactory.getLogger(AbstractMXBean.class); - public static String BASE_JMX_PREFIX = "org.opendaylight.controller:"; + public static final String BASE_JMX_PREFIX = "org.opendaylight.controller:"; private final MBeanServer server = ManagementFactory.getPlatformMBeanServer(); diff --git a/opendaylight/md-sal/sal-common-util/src/main/java/org/opendaylight/controller/sal/common/util/Rpcs.java b/opendaylight/md-sal/sal-common-util/src/main/java/org/opendaylight/controller/sal/common/util/Rpcs.java index 7bb8c25e3d..3278cef198 100644 --- a/opendaylight/md-sal/sal-common-util/src/main/java/org/opendaylight/controller/sal/common/util/Rpcs.java +++ b/opendaylight/md-sal/sal-common-util/src/main/java/org/opendaylight/controller/sal/common/util/Rpcs.java @@ -8,7 +8,6 @@ package org.opendaylight.controller.sal.common.util; import com.google.common.collect.ImmutableList; -import java.io.Serializable; import java.util.Collection; import org.opendaylight.yangtools.concepts.Immutable; import org.opendaylight.yangtools.yang.common.RpcError; @@ -37,8 +36,7 @@ public final class Rpcs { return new RpcResultTO<>(successful, null, errors); } - private static class RpcResultTO implements RpcResult, Serializable, Immutable { - private static final long serialVersionUID = 1L; + private static class RpcResultTO implements RpcResult, Immutable { private final Collection errors; private final T result; private final boolean successful; diff --git a/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/md/sal/dom/api/DOMDataTreeCommitCohortRegistry.java b/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/md/sal/dom/api/DOMDataTreeCommitCohortRegistry.java index 15795ebe3b..d3858dcb28 100644 --- a/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/md/sal/dom/api/DOMDataTreeCommitCohortRegistry.java +++ b/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/md/sal/dom/api/DOMDataTreeCommitCohortRegistry.java @@ -7,11 +7,14 @@ */ package org.opendaylight.controller.md.sal.dom.api; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + /** * Interface for a DOM commit cohort registry. * * @author Thomas Pantelis */ +@SuppressFBWarnings("NM_SAME_SIMPLE_NAME_AS_INTERFACE") public interface DOMDataTreeCommitCohortRegistry extends DOMDataBrokerExtension, org.opendaylight.mdsal.dom.api.DOMDataTreeCommitCohortRegistry { } diff --git a/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/md/sal/dom/api/DOMRpcIdentifier.java b/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/md/sal/dom/api/DOMRpcIdentifier.java index 5ead6b7593..60fae6781e 100644 --- a/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/md/sal/dom/api/DOMRpcIdentifier.java +++ b/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/md/sal/dom/api/DOMRpcIdentifier.java @@ -100,7 +100,7 @@ public abstract class DOMRpcIdentifier { final int prime = 31; int result = 1; result = prime * result + type.hashCode(); - result = prime * result + (getContextReference() == null ? 0 : getContextReference().hashCode()); + result = prime * result + getContextReference().hashCode(); return result; } diff --git a/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/sal/core/api/AbstractConsumer.java b/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/sal/core/api/AbstractConsumer.java index 6465e1deb2..46e9cdcd71 100644 --- a/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/sal/core/api/AbstractConsumer.java +++ b/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/sal/core/api/AbstractConsumer.java @@ -29,13 +29,14 @@ public abstract class AbstractConsumer implements Consumer, BundleActivator,Serv tracker.open(); } - - @Override public final void stop(final BundleContext bundleContext) throws Exception { stopImpl(bundleContext); broker = null; - tracker.close(); + + if (tracker != null) { + tracker.close(); + } } protected void startImpl(final BundleContext bundleContext) { @@ -54,7 +55,7 @@ public abstract class AbstractConsumer implements Consumer, BundleActivator,Serv @Override public Broker addingService(final ServiceReference reference) { - if (broker == null) { + if (broker == null && context != null) { broker = context.getService(reference); broker.registerConsumer(this, context); return broker; diff --git a/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/sal/core/api/AbstractProvider.java b/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/sal/core/api/AbstractProvider.java index 5cdc26d35e..b621e5f989 100644 --- a/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/sal/core/api/AbstractProvider.java +++ b/opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/sal/core/api/AbstractProvider.java @@ -45,14 +45,18 @@ public abstract class AbstractProvider implements BundleActivator, Provider,Serv @Override public final void stop(final BundleContext bundleContext) throws Exception { broker = null; - tracker.close(); + + if (tracker != null) { + tracker.close(); + } + tracker = null; stopImpl(bundleContext); } @Override public Broker addingService(final ServiceReference reference) { - if (broker == null) { + if (broker == null && context != null) { broker = context.getService(reference); broker.registerProvider(this, context); return broker; diff --git a/opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/md/sal/dom/spi/DefaultDOMRpcResult.java b/opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/md/sal/dom/spi/DefaultDOMRpcResult.java index 269fd3537c..94cc510645 100644 --- a/opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/md/sal/dom/spi/DefaultDOMRpcResult.java +++ b/opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/md/sal/dom/spi/DefaultDOMRpcResult.java @@ -9,6 +9,7 @@ package org.opendaylight.controller.md.sal.dom.spi; import com.google.common.annotations.Beta; import com.google.common.base.Preconditions; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.Serializable; import java.util.Arrays; import java.util.Collection; @@ -26,7 +27,15 @@ import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; @Beta public final class DefaultDOMRpcResult implements DOMRpcResult, Immutable, Serializable { private static final long serialVersionUID = 1L; + + // Flagged as "Non-transient non-serializable instance field" - the Collection is Serializable but the RpcError + // interface isn't. In lieu of changing the interface, we assume the implementation is Serializable which is + // reasonable since the only implementation that is actually used is from the RpcResultBuilder. + @SuppressFBWarnings("SE_BAD_FIELD") private final Collection errors; + + // Unfortunately the NormalizedNode interface isn't Serializable but we assume the implementations are. + @SuppressFBWarnings("SE_BAD_FIELD") private final NormalizedNode result; private static Collection asCollection(final RpcError... errors) { diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMImmutableDataChangeEvent.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMImmutableDataChangeEvent.java index 43b1c636e4..e47d535113 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMImmutableDataChangeEvent.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMImmutableDataChangeEvent.java @@ -133,8 +133,7 @@ public final class DOMImmutableDataChangeEvent implements private final Set removed = new HashSet<>(); private Builder(final DataChangeScope scope) { - Preconditions.checkNotNull(scope, "Data change scope should not be null."); - this.scope = scope; + this.scope = Preconditions.checkNotNull(scope, "Data change scope should not be null."); } public Builder setAfter(final NormalizedNode node) { diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java index af20e14c1c..f18bcfd788 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java @@ -67,9 +67,7 @@ public class InMemoryDOMDataStore extends TransactionReadyPrototype (listener, notification) -> { final AsyncDataChangeListener> inst = listener.getInstance(); - if (inst != null) { - inst.onDataChanged(notification); - } + inst.onDataChanged(notification); }; private final DataTree dataTree; diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMStoreTreeChangePublisher.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMStoreTreeChangePublisher.java index bf125cf33c..4b656400a8 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMStoreTreeChangePublisher.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMStoreTreeChangePublisher.java @@ -31,10 +31,7 @@ final class InMemoryDOMStoreTreeChangePublisher extends AbstractDOMStoreTreeChan private static final Invoker, DataTreeCandidate> MANAGER_INVOKER = (listener, notification) -> { // FIXME: this is inefficient, as we could grab the entire queue for the listener and post it - final DOMDataTreeChangeListener inst = listener.getInstance(); - if (inst != null) { - inst.onDataTreeChanged(Collections.singletonList(notification)); - } + listener.getInstance().onDataTreeChanged(Collections.singletonList(notification)); }; private static final Logger LOG = LoggerFactory.getLogger(InMemoryDOMStoreTreeChangePublisher.class); diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/ListenerNode.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/ListenerNode.java index 1928ec9670..00be7d68b7 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/ListenerNode.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/ListenerNode.java @@ -67,7 +67,11 @@ public class ListenerNode implements StoreTreeNode, Identifiable