From 6cd5778f454ba882d0e15361dfa6a5cd06721d97 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Thu, 8 Mar 2018 17:41:36 -0500 Subject: [PATCH] Fix findbugs violations in md-sal - part 2 - sal-broker-impl - sal-dom-broker-config - sal-binding-config - sal-akka-raft-example - clustering-it-provider Violations: - Method may return null, but is declared @Nonnull - Method with Optional return type returns explicit null - Method ignores exceptional return value - Field not guarded against concurrent access - Unchecked/unconfirmed cast of return value from method - Load of known null value - Parameter must be non-null but is marked as nullable - Class implements same interface as superclass - Redundant nullcheck of value known to be non-null - Unread field - Reliance on default encoding - Should be a static inner class - Questionable cast to concrete collection - Dead store to local variable - Dereference of the result of readLine() without nullcheck - Method ignores return value - Finalizer does not call superclass finalizer - An increment to a volatile field isn't atomic - Dead store to local variable - Redundant nullcheck of value known to be non-null Change-Id: Iec7205b49a0cbafff33db97c9c753a5425f929a6 Signed-off-by: Tom Pantelis --- .../cluster/example/ExampleActor.java | 2 +- .../example/ExampleRoleChangeListener.java | 8 +- .../cluster/example/LogGenerator.java | 2 +- .../controller/cluster/example/Main.java | 6 +- .../cluster/example/TestDriver.java | 6 +- .../md-sal/sal-binding-config/pom.xml | 7 + .../md-sal/sal-dom-broker-config/pom.xml | 7 + .../broker/impl/AbstractDOMDataBroker.java | 4 +- .../broker/impl/DOMNotificationRouter.java | 4 +- .../impl/DOMNotificationRouterEvent.java | 10 +- .../sal/dom/broker/impl/PingPongFuture.java | 7 +- ...ransactionCommitFailedExceptionMapper.java | 9 +- ...aBrokerDelegatingReadWriteTransaction.java | 4 +- .../LegacyEntityOwnershipServiceAdapter.java | 7 +- .../sal/dom/broker/ConsumerContextImpl.java | 33 +-- .../sal/dom/broker/osgi/ProxyFactory.java | 21 +- .../sal/dom/broker/util/YangSchemaUtils.java | 236 ------------------ .../clustering/it/provider/CarProvider.java | 26 +- .../provider/MdsalLowLevelTestProvider.java | 6 +- .../FinalizableScheduledExecutorService.java | 1 + .../impl/FlappingSingletonService.java | 15 +- 21 files changed, 102 insertions(+), 319 deletions(-) delete mode 100644 opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/util/YangSchemaUtils.java diff --git a/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/ExampleActor.java b/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/ExampleActor.java index 11278ede10..a1916f1500 100644 --- a/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/ExampleActor.java +++ b/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/ExampleActor.java @@ -151,7 +151,7 @@ public class ExampleActor extends RaftActor implements RaftActorRecoveryCohort, state.putAll(((MapState)snapshotState).state); if (LOG.isDebugEnabled()) { - LOG.debug("Snapshot applied to state : {}", ((HashMap) state).size()); + LOG.debug("Snapshot applied to state : {}", state.size()); } } diff --git a/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/ExampleRoleChangeListener.java b/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/ExampleRoleChangeListener.java index 0d11d7201d..f457074a7b 100644 --- a/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/ExampleRoleChangeListener.java +++ b/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/ExampleRoleChangeListener.java @@ -88,13 +88,11 @@ public class ExampleRoleChangeListener extends AbstractUntypedActor implements A } private void populateRegistry(String memberName) { - for (String shard: SHARDS_TO_MONITOR) { - String notifier = new StringBuilder().append(NOTIFIER_AKKA_URL).append(memberName) + String notifier = new StringBuilder().append(NOTIFIER_AKKA_URL).append(memberName) .append("/").append(memberName).append("-notifier").toString(); - if (!notifierRegistrationStatus.containsKey(notifier)) { - notifierRegistrationStatus.put(notifier, false); - } + if (!notifierRegistrationStatus.containsKey(notifier)) { + notifierRegistrationStatus.put(notifier, false); } if (!registrationSchedule.isCancelled()) { diff --git a/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/LogGenerator.java b/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/LogGenerator.java index a3fe498e8d..96712eef41 100644 --- a/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/LogGenerator.java +++ b/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/LogGenerator.java @@ -31,7 +31,7 @@ public class LogGenerator { clientToLoggingThread.remove(client); } - public class LoggingThread implements Runnable { + public static class LoggingThread implements Runnable { private final ActorRef clientActor; private volatile boolean stopLogging = false; diff --git a/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/Main.java b/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/Main.java index 5c3b49f247..54ff92993a 100644 --- a/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/Main.java +++ b/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/Main.java @@ -14,6 +14,7 @@ import akka.actor.PoisonPill; import com.google.common.base.Optional; import java.io.BufferedReader; import java.io.InputStreamReader; +import java.nio.charset.Charset; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -54,7 +55,7 @@ public final class Main { ActorRef clientActor = ACTOR_SYSTEM.actorOf(ClientActor.props(example1Actor)); BufferedReader br = - new BufferedReader(new InputStreamReader(System.in)); + new BufferedReader(new InputStreamReader(System.in, Charset.defaultCharset())); System.out.println("Usage :"); System.out.println("s <1-3> to start a peer"); @@ -64,6 +65,9 @@ public final class Main { System.out.print("Enter command (0 to exit):"); try { String line = br.readLine(); + if (line == null) { + continue; + } String[] split = line.split(" "); if (split.length > 1) { String command = split[0]; diff --git a/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/TestDriver.java b/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/TestDriver.java index ab980f7783..8f414c7a6b 100644 --- a/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/TestDriver.java +++ b/opendaylight/md-sal/sal-akka-raft-example/src/main/java/org/opendaylight/controller/cluster/example/TestDriver.java @@ -15,6 +15,7 @@ import com.google.common.collect.Lists; import com.typesafe.config.ConfigFactory; import java.io.BufferedReader; import java.io.InputStreamReader; +import java.nio.charset.Charset; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -75,9 +76,12 @@ public class TestDriver { System.out.println("Enter command (type bye to exit):"); - BufferedReader br = new BufferedReader(new InputStreamReader(System.in)); + BufferedReader br = new BufferedReader(new InputStreamReader(System.in, Charset.defaultCharset())); while (true) { String command = br.readLine(); + if (command == null) { + continue; + } if (command.startsWith("bye")) { System.exit(0); diff --git a/opendaylight/md-sal/sal-binding-config/pom.xml b/opendaylight/md-sal/sal-binding-config/pom.xml index 6caaa6d1f6..13bf461ade 100644 --- a/opendaylight/md-sal/sal-binding-config/pom.xml +++ b/opendaylight/md-sal/sal-binding-config/pom.xml @@ -12,6 +12,13 @@ 1.8.0-SNAPSHOT bundle + + + true + + org.opendaylight.controller diff --git a/opendaylight/md-sal/sal-dom-broker-config/pom.xml b/opendaylight/md-sal/sal-dom-broker-config/pom.xml index 55890c0edc..f0ed089819 100644 --- a/opendaylight/md-sal/sal-dom-broker-config/pom.xml +++ b/opendaylight/md-sal/sal-dom-broker-config/pom.xml @@ -12,6 +12,13 @@ 1.8.0-SNAPSHOT bundle + + + true + + org.opendaylight.controller diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/AbstractDOMDataBroker.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/AbstractDOMDataBroker.java index f2caddc718..2959a2825a 100755 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/AbstractDOMDataBroker.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/AbstractDOMDataBroker.java @@ -33,8 +33,8 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public abstract class AbstractDOMDataBroker extends AbstractDOMForwardedTransactionFactory implements - DOMDataBroker, AutoCloseable { +public abstract class AbstractDOMDataBroker extends AbstractDOMForwardedTransactionFactory + implements DOMDataBroker { private static final Logger LOG = LoggerFactory.getLogger(AbstractDOMDataBroker.class); private final AtomicLong txNum = new AtomicLong(); diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMNotificationRouter.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMNotificationRouter.java index bb2275f1fb..95d9e2171b 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMNotificationRouter.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMNotificationRouter.java @@ -157,7 +157,7 @@ public final class DOMNotificationRouter implements AutoCloseable, DOMNotificati private void notifyListenerTypesChanged(final Set typesAfter) { final List> listenersAfter = ImmutableList .copyOf(subscriptionListeners.getListeners()); - executor.submit(() -> { + executor.execute(() -> { for (final ListenerRegistration subListener : listenersAfter) { try { subListener.getInstance().onSubscriptionChanged(typesAfter); @@ -172,7 +172,7 @@ public final class DOMNotificationRouter implements AutoCloseable, DOMNotificati public ListenerRegistration registerSubscriptionListener( final L listener) { final Set initialTypes = listeners.keySet(); - executor.submit(() -> listener.onSubscriptionChanged(initialTypes)); + executor.execute(() -> listener.onSubscriptionChanged(initialTypes)); return subscriptionListeners.registerWithType(listener); } diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMNotificationRouterEvent.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMNotificationRouterEvent.java index a1d5faee7c..8aa9f0d503 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMNotificationRouterEvent.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/DOMNotificationRouterEvent.java @@ -48,11 +48,9 @@ final class DOMNotificationRouterEvent { LOG.trace("Start delivery of notification {}", notification); for (ListenerRegistration r : subscribers) { final DOMNotificationListener listener = r.getInstance(); - if (listener != null) { - LOG.trace("Notifying listener {}", listener); - listener.onNotification(notification); - LOG.trace("Listener notification completed"); - } + LOG.trace("Notifying listener {}", listener); + listener.onNotification(notification); + LOG.trace("Listener notification completed"); } LOG.trace("Delivery completed"); } @@ -63,4 +61,4 @@ final class DOMNotificationRouterEvent { subscribers = null; future = null; } -} \ No newline at end of file +} diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/PingPongFuture.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/PingPongFuture.java index fd8f93b67d..63aba6e5f7 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/PingPongFuture.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/PingPongFuture.java @@ -22,10 +22,11 @@ final class PingPongFuture extends AbstractCheckedFuture currentHistory = Lists.newLinkedList(modificationHistoryMap.get(store)); Futures.addCallback(initialReadMap.get(store), new FutureCallback>>() { @Override - public void onSuccess(@Nullable final Optional> result) { + public void onSuccess(@Nonnull final Optional> result) { final DataTreeModification mod = snapshotMap.get(store).newModification(); if (result.isPresent()) { mod.write(path, result.get()); diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/clustering/impl/LegacyEntityOwnershipServiceAdapter.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/clustering/impl/LegacyEntityOwnershipServiceAdapter.java index 5d91a742a3..c8a59f3419 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/clustering/impl/LegacyEntityOwnershipServiceAdapter.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/clustering/impl/LegacyEntityOwnershipServiceAdapter.java @@ -9,6 +9,7 @@ package org.opendaylight.controller.md.sal.dom.clustering.impl; import com.google.common.base.Optional; import com.google.common.base.Preconditions; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import javax.annotation.Nonnull; import org.opendaylight.controller.md.sal.common.api.clustering.CandidateAlreadyRegisteredException; import org.opendaylight.controller.md.sal.common.api.clustering.Entity; @@ -135,9 +136,11 @@ public class LegacyEntityOwnershipServiceAdapter implements EntityOwnershipServi } @Override + @SuppressFBWarnings("BC_UNCONFIRMED_CAST_OF_RETURN_VALUE") public void ownershipChanged(final DOMEntityOwnershipChange ownershipChange) { - Entity entity = new Entity(ownershipChange.getEntity().getType(), - ownershipChange.getEntity().getIdentifier()); + final DOMEntity domEntity = ownershipChange.getEntity(); + Entity entity = new Entity(domEntity.getType(), + domEntity.getIdentifier()); delegateListener.ownershipChanged(new EntityOwnershipChange(entity, ownershipChange.getState().wasOwner(), ownershipChange.getState().isOwner(), ownershipChange.getState().hasOwner(), diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/ConsumerContextImpl.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/ConsumerContextImpl.java index 00cc8dc51e..a4f67df0f0 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/ConsumerContextImpl.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/ConsumerContextImpl.java @@ -12,7 +12,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ClassToInstanceMap; import com.google.common.collect.MutableClassToInstanceMap; import java.util.Collection; -import javax.annotation.concurrent.GuardedBy; +import java.util.concurrent.atomic.AtomicBoolean; import org.opendaylight.controller.sal.core.api.Broker.ConsumerSession; import org.opendaylight.controller.sal.core.api.BrokerService; import org.opendaylight.controller.sal.core.api.Consumer; @@ -24,9 +24,8 @@ class ConsumerContextImpl implements ConsumerSession { private final ClassToInstanceMap instantiatedServices = MutableClassToInstanceMap.create(); private final Consumer consumer; - private BrokerImpl broker = null; - @GuardedBy("this") - private boolean closed = false; + private final BrokerImpl broker; + private final AtomicBoolean closed = new AtomicBoolean(false); ConsumerContextImpl(final Consumer provider, final BrokerImpl brokerImpl) { broker = brokerImpl; @@ -52,27 +51,21 @@ class ConsumerContextImpl implements ConsumerSession { @Override public void close() { - synchronized (this) { - if (closed) { - return; + if (closed.compareAndSet(false, true)) { + Collection toStop = instantiatedServices.values(); + for (BrokerService brokerService : toStop) { + if (brokerService instanceof AbstractBrokerServiceProxy) { + ((AbstractBrokerServiceProxy) brokerService).close(); + } } - this.closed = true; + broker.consumerSessionClosed(this); } - - Collection toStop = instantiatedServices.values(); - for (BrokerService brokerService : toStop) { - if (brokerService instanceof AbstractBrokerServiceProxy) { - ((AbstractBrokerServiceProxy) brokerService).close(); - } - } - broker.consumerSessionClosed(this); - broker = null; } @Override - public synchronized boolean isClosed() { - return closed; + public boolean isClosed() { + return closed.get(); } /** @@ -95,6 +88,6 @@ class ConsumerContextImpl implements ConsumerSession { } protected final void checkNotClosed() { - Preconditions.checkState(!closed, "Session is closed."); + Preconditions.checkState(!closed.get(), "Session is closed."); } } diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/osgi/ProxyFactory.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/osgi/ProxyFactory.java index f922882f7f..1ca9f5da96 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/osgi/ProxyFactory.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/osgi/ProxyFactory.java @@ -7,7 +7,6 @@ */ package org.opendaylight.controller.sal.dom.broker.osgi; -import java.util.Arrays; import org.opendaylight.controller.md.sal.dom.api.DOMDataBroker; import org.opendaylight.controller.md.sal.dom.api.DOMMountPointService; import org.opendaylight.controller.sal.core.api.BrokerService; @@ -23,25 +22,28 @@ public final class ProxyFactory { public static T createProxy(final ServiceReference serviceRef, final T service) { Object createProxyImpl = ProxyFactory.createProxyImpl(serviceRef, service); - return ((T) createProxyImpl); + return (T) createProxyImpl; } private static Object createProxyImpl(final ServiceReference ref, final DOMMountPointService service) { - return new DOMMountPointServiceProxy(((ServiceReference) ref), service); + return new DOMMountPointServiceProxy((ServiceReference) ref, service); } private static Object createProxyImpl(final ServiceReference ref, final SchemaService service) { - return new SchemaServiceProxy(((ServiceReference) ref), service); + return new SchemaServiceProxy((ServiceReference) ref, service); } private static DOMDataBrokerProxy createProxyImpl(final ServiceReference ref, final DOMDataBroker service) { - return new DOMDataBrokerProxy(((ServiceReference) ref), service); + return new DOMDataBrokerProxy((ServiceReference) ref, service); } private static Object createProxyImpl(final ServiceReference ref, final BrokerService service) { + if (service == null) { + throw new IllegalArgumentException("service can't be null"); + } if (service instanceof DOMDataBroker) { return createProxyImpl(ref, (DOMDataBroker) service); @@ -49,16 +51,13 @@ public final class ProxyFactory { return createProxyImpl(ref, (SchemaService) service); } else if (service instanceof DOMMountPointService) { return createProxyImpl(ref, (DOMMountPointService) service); - } else if (service != null) { - return createProxyImplFallback(ref, service); - } else { - throw new IllegalArgumentException( - "Unhandled parameter types: " + Arrays.asList(ref, service).toString()); } + + return createProxyImplFallback(ref, service); } private static Object createProxyImplFallback(final ServiceReference reference, final BrokerService service) { return service; } -} \ No newline at end of file +} diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/util/YangSchemaUtils.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/util/YangSchemaUtils.java deleted file mode 100644 index 7aa67a1116..0000000000 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/util/YangSchemaUtils.java +++ /dev/null @@ -1,236 +0,0 @@ -/* - * Copyright (c) 2014 Cisco Systems, Inc. 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.sal.dom.broker.util; - -import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkState; - -import com.google.common.collect.Lists; -import java.util.Collection; -import java.util.Iterator; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import org.opendaylight.yangtools.yang.common.QName; -import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; -import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; -import org.opendaylight.yangtools.yang.model.api.ActionDefinition; -import org.opendaylight.yangtools.yang.model.api.AugmentationSchemaNode; -import org.opendaylight.yangtools.yang.model.api.CaseSchemaNode; -import org.opendaylight.yangtools.yang.model.api.ChoiceSchemaNode; -import org.opendaylight.yangtools.yang.model.api.ContainerSchemaNode; -import org.opendaylight.yangtools.yang.model.api.DataNodeContainer; -import org.opendaylight.yangtools.yang.model.api.DataSchemaNode; -import org.opendaylight.yangtools.yang.model.api.GroupingDefinition; -import org.opendaylight.yangtools.yang.model.api.LeafListSchemaNode; -import org.opendaylight.yangtools.yang.model.api.LeafSchemaNode; -import org.opendaylight.yangtools.yang.model.api.MustDefinition; -import org.opendaylight.yangtools.yang.model.api.NotificationDefinition; -import org.opendaylight.yangtools.yang.model.api.RevisionAwareXPath; -import org.opendaylight.yangtools.yang.model.api.SchemaContext; -import org.opendaylight.yangtools.yang.model.api.SchemaPath; -import org.opendaylight.yangtools.yang.model.api.Status; -import org.opendaylight.yangtools.yang.model.api.TypeDefinition; -import org.opendaylight.yangtools.yang.model.api.UnknownSchemaNode; -import org.opendaylight.yangtools.yang.model.api.UsesNode; - -public final class YangSchemaUtils { - private YangSchemaUtils() { - throw new UnsupportedOperationException("Utility class."); - } - - public static DataSchemaNode getSchemaNode(final SchemaContext schema, final YangInstanceIdentifier path) { - checkArgument(schema != null, "YANG Schema must not be null."); - checkArgument(path != null, "Path must not be null."); - return getSchemaNode(schema, Lists.transform(path.getPathArguments(), PathArgument::getNodeType)); - } - - public static DataSchemaNode getSchemaNode(final SchemaContext schema, final Iterable path) { - checkArgument(schema != null, "YANG Schema must not be null."); - checkArgument(path != null, "Path must not be null."); - if (!path.iterator().hasNext()) { - return toRootDataNode(schema); - } - - QName firstNode = path.iterator().next(); - DataNodeContainer previous = schema.findModule(firstNode.getModule()).orElse(null); - Iterator iterator = path.iterator(); - - while (iterator.hasNext()) { - checkArgument(previous != null, "Supplied path does not resolve into valid schema node."); - QName arg = iterator.next(); - DataSchemaNode currentNode = previous.getDataChildByName(arg); - if (currentNode == null) { - currentNode = searchInChoices(previous, arg); - } - if (currentNode instanceof DataNodeContainer) { - previous = (DataNodeContainer) currentNode; - } else if (currentNode instanceof LeafSchemaNode || currentNode instanceof LeafListSchemaNode) { - checkArgument(!iterator.hasNext(), "Path nests inside leaf node, which is not allowed."); - return currentNode; - } - checkState(currentNode != null, "Current node should not be null for %s", path); - } - checkState(previous instanceof DataSchemaNode, - "Schema node for %s should be instance of DataSchemaNode. Found %s", path, previous); - return (DataSchemaNode) previous; - } - - private static DataSchemaNode searchInChoices(final DataNodeContainer node, final QName arg) { - for (DataSchemaNode child : node.getChildNodes()) { - if (child instanceof ChoiceSchemaNode) { - ChoiceSchemaNode choiceNode = (ChoiceSchemaNode) child; - DataSchemaNode potential = searchInCases(choiceNode, arg); - if (potential != null) { - return potential; - } - } - } - return null; - } - - private static DataSchemaNode searchInCases(final ChoiceSchemaNode choiceNode, final QName arg) { - for (CaseSchemaNode caseNode : choiceNode.getCases().values()) { - DataSchemaNode node = caseNode.getDataChildByName(arg); - if (node != null) { - return node; - } - } - return null; - } - - private static ContainerSchemaNode toRootDataNode(final SchemaContext schema) { - return new NetconfDataRootNode(schema); - } - - private static final class NetconfDataRootNode implements ContainerSchemaNode { - - NetconfDataRootNode(final SchemaContext schema) { - // TODO Auto-generated constructor stub - } - - @Override - public Set> getTypeDefinitions() { - // TODO Auto-generated method stub - return null; - } - - @Override - public Set getChildNodes() { - // TODO Auto-generated method stub - return null; - } - - @Override - public Set getGroupings() { - // TODO Auto-generated method stub - return null; - } - - @Override - public Optional findDataChildByName(final QName name) { - // TODO Auto-generated method stub - return null; - } - - @Override - public Set getUses() { - // TODO Auto-generated method stub - return null; - } - - @Override - public Set getAvailableAugmentations() { - // TODO Auto-generated method stub - return null; - } - - @Override - public boolean isAugmenting() { - // TODO Auto-generated method stub - return false; - } - - @Override - public boolean isAddedByUses() { - // TODO Auto-generated method stub - return false; - } - - @Override - public boolean isConfiguration() { - // TODO Auto-generated method stub - return false; - } - - @Override - public QName getQName() { - // TODO Auto-generated method stub - return null; - } - - @Override - public SchemaPath getPath() { - // TODO Auto-generated method stub - return null; - } - - @Override - public Optional getDescription() { - return Optional.empty(); - } - - @Override - public Optional getReference() { - return Optional.empty(); - } - - @Override - public Status getStatus() { - // TODO Auto-generated method stub - return null; - } - - @Override - public List getUnknownSchemaNodes() { - // TODO Auto-generated method stub - return null; - } - - @Override - public boolean isPresenceContainer() { - // TODO Auto-generated method stub - return false; - } - - @Override - public Set getNotifications() { - // TODO Auto-generated method stub - return null; - } - - @Override - public Set getActions() { - // TODO Auto-generated method stub - return null; - } - - @Override - public Optional getWhenCondition() { - // TODO Auto-generated method stub - return null; - } - - @Override - public Collection getMustConstraints() { - // TODO Auto-generated method stub - return null; - } - } - -} diff --git a/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/CarProvider.java b/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/CarProvider.java index 33ec2ed7d6..857a61b18c 100644 --- a/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/CarProvider.java +++ b/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/CarProvider.java @@ -96,7 +96,7 @@ public class CarProvider implements CarService { public void close() { stopThread(); - unregisterCommitCohort(); + closeCommitCohortRegistration(); } private void stopThread() { @@ -254,11 +254,8 @@ public class CarProvider implements CarService { final ListenerRegistration carsDtclRegistration = dataProvider.registerDataTreeChangeListener(CARS_DTID, new CarDataTreeChangeListener()); - if (carsDtclRegistration != null) { - carsDtclRegistrations.add(carsDtclRegistration); - return RpcResultBuilder.success().buildFuture(); - } - return RpcResultBuilder.failed().buildFuture(); + carsDtclRegistrations.add(carsDtclRegistration); + return RpcResultBuilder.success().buildFuture(); } @Override @@ -279,18 +276,17 @@ public class CarProvider implements CarService { @Override @SuppressWarnings("checkstyle:IllegalCatch") public Future> unregisterCommitCohort() { + closeCommitCohortRegistration(); + + return RpcResultBuilder.success().buildFuture(); + } + + private void closeCommitCohortRegistration() { final DOMDataTreeCommitCohortRegistration reg = commitCohortReg.getAndSet(null); if (reg != null) { - try { - reg.close(); - LOG_CAR_PROVIDER.info("Unregistered commit cohort"); - } catch (Exception e) { - return RpcResultBuilder.failed().withError(ErrorType.APPLICATION, - "Error closing commit cohort registration", e).buildFuture(); - } + reg.close(); + LOG_CAR_PROVIDER.info("Unregistered commit cohort"); } - - return RpcResultBuilder.success().buildFuture(); } @Override diff --git a/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/MdsalLowLevelTestProvider.java b/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/MdsalLowLevelTestProvider.java index c5427bef59..ec3da0ef36 100644 --- a/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/MdsalLowLevelTestProvider.java +++ b/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/MdsalLowLevelTestProvider.java @@ -21,8 +21,6 @@ import com.google.common.base.Strings; import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.SettableFuture; -import java.io.PrintWriter; -import java.io.StringWriter; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -552,10 +550,8 @@ public class MdsalLowLevelTestProvider implements OdlMdsalLowlevelControlService new CheckPublishNotificationsOutputBuilder().setActive(!task.isFinished()); if (task.getLastError() != null) { - final StringWriter sw = new StringWriter(); - final PrintWriter pw = new PrintWriter(sw); LOG.error("Last error for {}", task, task.getLastError()); - checkPublishNotificationsOutputBuilder.setLastError(task.getLastError().toString() + sw.toString()); + checkPublishNotificationsOutputBuilder.setLastError(task.getLastError().toString()); } final CheckPublishNotificationsOutput output = diff --git a/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/FinalizableScheduledExecutorService.java b/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/FinalizableScheduledExecutorService.java index 758185346c..f0873034b4 100644 --- a/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/FinalizableScheduledExecutorService.java +++ b/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/FinalizableScheduledExecutorService.java @@ -32,6 +32,7 @@ final class FinalizableScheduledExecutorService extends ScheduledThreadPoolExecu @Override @SuppressWarnings("checkstyle:NoFinalizer") protected void finalize() { + super.finalize(); super.shutdownNow(); } } diff --git a/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/FlappingSingletonService.java b/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/FlappingSingletonService.java index 2894ee3598..3ca46b128f 100644 --- a/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/FlappingSingletonService.java +++ b/opendaylight/md-sal/samples/clustering-test-app/provider/src/main/java/org/opendaylight/controller/clustering/it/provider/impl/FlappingSingletonService.java @@ -13,6 +13,7 @@ import com.google.common.util.concurrent.ListenableFuture; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonService; import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider; import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceRegistration; @@ -32,7 +33,7 @@ public class FlappingSingletonService implements ClusterSingletonService { private final ClusterSingletonServiceProvider singletonServiceProvider; private final AtomicBoolean active = new AtomicBoolean(true); - private volatile long flapCount = 0; + private final AtomicLong flapCount = new AtomicLong(); private volatile ClusterSingletonServiceRegistration registration; public FlappingSingletonService(final ClusterSingletonServiceProvider singletonServiceProvider) { @@ -56,7 +57,9 @@ public class FlappingSingletonService implements ClusterSingletonService { } catch (Exception e) { LOG.warn("There was a problem closing flapping singleton service.", e); setInactive(); - flapCount = -flapCount; + + final long count = flapCount.get(); + flapCount.compareAndSet(count, -count); } }); } @@ -66,7 +69,7 @@ public class FlappingSingletonService implements ClusterSingletonService { public ListenableFuture closeServiceInstance() { LOG.debug("Closing flapping-singleton-service, flapCount: {}", flapCount); - flapCount++; + flapCount.incrementAndGet(); if (active.get()) { // TODO direct registration/close seem to trigger a bug in singleton state transitions, // remove whole executor shenanigans after it's fixed. @@ -78,7 +81,9 @@ public class FlappingSingletonService implements ClusterSingletonService { } catch (RuntimeException e) { LOG.warn("There was a problem re-registering flapping singleton service.", e); setInactive(); - flapCount = -flapCount - 1; + + final long count = flapCount.get(); + flapCount.compareAndSet(count, -count - 1); } }, 200, TimeUnit.MILLISECONDS); @@ -96,6 +101,6 @@ public class FlappingSingletonService implements ClusterSingletonService { LOG.debug("Setting flapping-singleton-service to inactive, flap-count: {}", flapCount); active.set(false); - return flapCount; + return flapCount.get(); } } -- 2.36.6