Fix findbugs violations in md-sal - part 1 26/69326/3
authorTom Pantelis <tompantelis@gmail.com>
Thu, 8 Mar 2018 02:54:09 +0000 (21:54 -0500)
committerMichael Vorburger <vorburger@redhat.com>
Thu, 15 Mar 2018 14:27:18 +0000 (14:27 +0000)
- 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 <tompantelis@gmail.com>
13 files changed:
opendaylight/md-sal/sal-binding-api/src/main/java/org/opendaylight/controller/sal/binding/api/AbstractBrokerAwareActivator.java
opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationOperation.java
opendaylight/md-sal/sal-common-util/src/main/java/org/opendaylight/controller/md/sal/common/util/jmx/AbstractMXBean.java
opendaylight/md-sal/sal-common-util/src/main/java/org/opendaylight/controller/sal/common/util/Rpcs.java
opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/md/sal/dom/api/DOMDataTreeCommitCohortRegistry.java
opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/md/sal/dom/api/DOMRpcIdentifier.java
opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/sal/core/api/AbstractConsumer.java
opendaylight/md-sal/sal-dom-api/src/main/java/org/opendaylight/controller/sal/core/api/AbstractProvider.java
opendaylight/md-sal/sal-dom-spi/src/main/java/org/opendaylight/controller/md/sal/dom/spi/DefaultDOMRpcResult.java
opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMImmutableDataChangeEvent.java
opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java
opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMStoreTreeChangePublisher.java
opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/tree/ListenerNode.java

index 83af462..7c29846 100644 (file)
@@ -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);
     }
 
index 4174fac..e86c1eb 100644 (file)
@@ -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<T extends PathArgument> impleme
         }
 
         @Override
+        @SuppressFBWarnings("BC_UNCONFIRMED_CAST")
         public NormalizedNode<?, ?> createDefault(final PathArgument currentArg) {
             final DataContainerNodeAttrBuilder<NodeIdentifierWithPredicates, MapEntryNode> builder = Builders
                     .mapEntryBuilder().withNodeIdentifier((NodeIdentifierWithPredicates) currentArg);
@@ -249,6 +251,7 @@ public abstract class DataNormalizationOperation<T extends PathArgument> 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<T extends PathArgument> impleme
         }
 
         @Override
+        @SuppressFBWarnings("BC_UNCONFIRMED_CAST")
         public NormalizedNode<?, ?> createDefault(final PathArgument currentArg) {
             return Builders.containerBuilder().withNodeIdentifier((NodeIdentifier) currentArg).build();
         }
index d60623a..3cf6d9f 100644 (file)
@@ -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();
 
index 7bb8c25..3278cef 100644 (file)
@@ -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<T> implements RpcResult<T>, Serializable, Immutable {
-        private static final long serialVersionUID = 1L;
+    private static class RpcResultTO<T> implements RpcResult<T>, Immutable {
         private final Collection<RpcError> errors;
         private final T result;
         private final boolean successful;
index 15795eb..d3858dc 100644 (file)
@@ -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 {
 }
index 5ead6b7..60fae67 100644 (file)
@@ -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;
     }
 
index 6465e1d..46e9cdc 100644 (file)
@@ -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<Broker> reference) {
-        if (broker == null) {
+        if (broker == null && context != null) {
             broker = context.getService(reference);
             broker.registerConsumer(this, context);
             return broker;
index 5cdc26d..b621e5f 100644 (file)
@@ -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<Broker> reference) {
-        if (broker == null) {
+        if (broker == null && context != null) {
             broker = context.getService(reference);
             broker.registerProvider(this, context);
             return broker;
index 269fd35..94cc510 100644 (file)
@@ -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<RpcError> 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<RpcError> asCollection(final RpcError... errors) {
index 43b1c63..e47d535 100644 (file)
@@ -133,8 +133,7 @@ public final class DOMImmutableDataChangeEvent implements
         private final Set<YangInstanceIdentifier> 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) {
index af20e14..f18bcfd 100644 (file)
@@ -67,9 +67,7 @@ public class InMemoryDOMDataStore extends TransactionReadyPrototype<String>
             (listener, notification) -> {
                 final AsyncDataChangeListener<YangInstanceIdentifier, NormalizedNode<?, ?>> inst =
                         listener.getInstance();
-                if (inst != null) {
-                    inst.onDataChanged(notification);
-                }
+                inst.onDataChanged(notification);
             };
 
     private final DataTree dataTree;
index bf125cf..4b65640 100644 (file)
@@ -31,10 +31,7 @@ final class InMemoryDOMStoreTreeChangePublisher extends AbstractDOMStoreTreeChan
     private static final Invoker<AbstractDOMDataTreeChangeListenerRegistration<?>, 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);
index 1928ec9..00be7d6 100644 (file)
@@ -67,7 +67,11 @@ public class ListenerNode implements StoreTreeNode<ListenerNode>, Identifiable<P
 
     @Override
     public boolean equals(final Object obj) {
-        return delegate.equals(obj);
+        if (obj == null || getClass() != obj.getClass()) {
+            return false;
+        }
+
+        return delegate.equals(((ListenerNode)obj).delegate);
     }
 
     @Override

©2013 OpenDaylight, A Linux Foundation Collaborative Project. All Rights Reserved.
OpenDaylight is a registered trademark of The OpenDaylight Project, Inc.
Linux Foundation and OpenDaylight are registered trademarks of the Linux Foundation.
Linux is a registered trademark of Linus Torvalds.