Minor sal-distributed-datastore cleanups

Corrent a @GuardedBy annotations, add @NonNull annotations, fix javadocs
and make a few methods final.

Change-Id: Icb93aae229fb0aece5ece223a759ce9f36ee0297
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHandle.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHandle.java
index 90971b31cece3b7718e830b6a59014405d0d8a7b..aa4779c898b5847e3dca9551173d3aa16a2a28fa 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHandle.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHandle.java
@@ -55,6 +55,7 @@ public abstract class AbstractClientHandle<T extends AbstractProxyTransaction> e
     }
 
     @Override
     }
 
     @Override

+    // Non-final for mocking

     public TransactionIdentifier getIdentifier() {
         return transactionId;
     }
     public TransactionIdentifier getIdentifier() {
         return transactionId;
     }


@@ -64,6 +65,7 @@ public abstract class AbstractClientHandle<T extends AbstractProxyTransaction> e
      *
      * @return True if this transaction became closed during this call
      */
      *
      * @return True if this transaction became closed during this call
      */

+    // Non-final for mocking

     public boolean abort() {
         if (commonAbort()) {
             parent.onTransactionAbort(this);
     public boolean abort() {
         if (commonAbort()) {
             parent.onTransactionAbort(this);

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHistory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHistory.java
index d445d3c2f6f9e542ec031c2737653317e3952a73..d306d13e2e29f50063b89f42136a5dffd4fc03ed 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHistory.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHistory.java
@@ -104,7 +104,7 @@ public abstract class AbstractClientHistory extends LocalAbortable implements Id
     }
 
     @Override
     }
 
     @Override

-    public LocalHistoryIdentifier getIdentifier() {
+    public final LocalHistoryIdentifier getIdentifier() {

         return identifier;
     }
 
         return identifier;
     }
 


@@ -135,6 +135,7 @@ public abstract class AbstractClientHistory extends LocalAbortable implements Id
     /**
      * Create a new history proxy for a given shard.
      *
     /**
      * Create a new history proxy for a given shard.
      *

+     * @param shard Shard cookie

      * @throws InversibleLockException if the shard is being reconnected
      */
     @Holding("lock")
      * @throws InversibleLockException if the shard is being reconnected
      */
     @Holding("lock")


@@ -203,6 +204,7 @@ public abstract class AbstractClientHistory extends LocalAbortable implements Id
      * @throws DOMTransactionChainClosedException if this history is closed
      * @throws IllegalStateException if a previous dependent transaction has not been closed
      */
      * @throws DOMTransactionChainClosedException if this history is closed
      * @throws IllegalStateException if a previous dependent transaction has not been closed
      */

+    // Non-final for mocking

     public @NonNull ClientTransaction createTransaction() {
         checkNotClosed();
 
     public @NonNull ClientTransaction createTransaction() {
         checkNotClosed();
 


@@ -220,6 +222,7 @@ public abstract class AbstractClientHistory extends LocalAbortable implements Id
      * @throws DOMTransactionChainClosedException if this history is closed
      * @throws IllegalStateException if a previous dependent transaction has not been closed
      */
      * @throws DOMTransactionChainClosedException if this history is closed
      * @throws IllegalStateException if a previous dependent transaction has not been closed
      */

+    // Non-final for mocking

     public ClientSnapshot takeSnapshot() {
         checkNotClosed();
 
     public ClientSnapshot takeSnapshot() {
         checkNotClosed();
 


@@ -239,7 +242,7 @@ public abstract class AbstractClientHistory extends LocalAbortable implements Id
     /**
      * Callback invoked from {@link ClientTransaction} when a child transaction readied for submission.
      *
     /**
      * Callback invoked from {@link ClientTransaction} when a child transaction readied for submission.
      *

-     * @param txId Transaction identifier
+     * @param tx Client transaction

      * @param cohort Transaction commit cohort
      */
     synchronized AbstractTransactionCommitCohort onTransactionReady(final ClientTransaction tx,
      * @param cohort Transaction commit cohort
      */
     synchronized AbstractTransactionCommitCohort onTransactionReady(final ClientTransaction tx,


@@ -274,13 +277,14 @@ public abstract class AbstractClientHistory extends LocalAbortable implements Id
      *
      * @param txId transaction identifier
      */
      *
      * @param txId transaction identifier
      */

+    // Non-final for mocking

     synchronized void onTransactionComplete(final TransactionIdentifier txId) {
         if (readyTransactions.remove(txId) == null) {
             LOG.warn("Could not find completed transaction {}", txId);
         }
     }
 
     synchronized void onTransactionComplete(final TransactionIdentifier txId) {
         if (readyTransactions.remove(txId) == null) {
             LOG.warn("Could not find completed transaction {}", txId);
         }
     }
 

-    HistoryReconnectCohort startReconnect(final ConnectedClientConnection<ShardBackendInfo> newConn) {
+    final HistoryReconnectCohort startReconnect(final ConnectedClientConnection<ShardBackendInfo> newConn) {

         /*
          * This looks ugly and unusual and there is a reason for that, as the locking involved is in multiple places.
          *
         /*
          * This looks ugly and unusual and there is a reason for that, as the locking involved is in multiple places.
          *


@@ -328,5 +332,4 @@ public abstract class AbstractClientHistory extends LocalAbortable implements Id
             }
         };
     }
             }
         };
     }

-

 }
 }

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientTransaction.java
index 9b4c5a962f9b62959ffc3464b475c27a7d198b59..f5855c279a9e4e9f648b93a047c1d9cdc3c4a1cc 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientTransaction.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientTransaction.java
@@ -7,9 +7,9 @@
  */
 package org.opendaylight.controller.cluster.databroker.actors.dds;
 
  */
 package org.opendaylight.controller.cluster.databroker.actors.dds;
 

+import static com.google.common.base.Preconditions.checkState;
+

 import com.google.common.annotations.Beta;
 import com.google.common.annotations.Beta;

-import com.google.common.base.Preconditions;
-import com.google.common.collect.Iterables;

 import com.google.common.util.concurrent.FluentFuture;
 import java.util.Collection;
 import java.util.Optional;
 import com.google.common.util.concurrent.FluentFuture;
 import java.util.Collection;
 import java.util.Optional;


@@ -80,7 +80,7 @@ public class ClientTransaction extends AbstractClientHandle<AbstractProxyTransac
 
     public DOMStoreThreePhaseCommitCohort ready() {
         final Collection<AbstractProxyTransaction> toReady = ensureClosed();
 
     public DOMStoreThreePhaseCommitCohort ready() {
         final Collection<AbstractProxyTransaction> toReady = ensureClosed();

-        Preconditions.checkState(toReady != null, "Attempted to submit a closed transaction %s", this);
+        checkState(toReady != null, "Attempted to submit a closed transaction %s", this);

 
         toReady.forEach(AbstractProxyTransaction::seal);
         final AbstractTransactionCommitCohort cohort;
 
         toReady.forEach(AbstractProxyTransaction::seal);
         final AbstractTransactionCommitCohort cohort;


@@ -89,8 +89,7 @@ public class ClientTransaction extends AbstractClientHandle<AbstractProxyTransac
                 cohort = new EmptyTransactionCommitCohort(parent(), getIdentifier());
                 break;
             case 1:
                 cohort = new EmptyTransactionCommitCohort(parent(), getIdentifier());
                 break;
             case 1:

-                cohort = new DirectTransactionCommitCohort(parent(), getIdentifier(),
-                    Iterables.getOnlyElement(toReady));
+                cohort = new DirectTransactionCommitCohort(parent(), getIdentifier(), toReady.iterator().next());

                 break;
             default:
                 cohort = new ClientTransactionCommitCohort(parent(), getIdentifier(), toReady);
                 break;
             default:
                 cohort = new ClientTransactionCommitCohort(parent(), getIdentifier(), toReady);

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ProxyHistory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ProxyHistory.java
index 92284e05a2fcef37edfa4362380279514f154d9c..fb8764041e66bb88ee476ed20183eca2152ee549 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ProxyHistory.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ProxyHistory.java
@@ -8,10 +8,11 @@
 package org.opendaylight.controller.cluster.databroker.actors.dds;
 
 import static com.google.common.base.Preconditions.checkState;
 package org.opendaylight.controller.cluster.databroker.actors.dds;
 
 import static com.google.common.base.Preconditions.checkState;

+import static com.google.common.base.Verify.verify;
+import static com.google.common.base.Verify.verifyNotNull;

 import static java.util.Objects.requireNonNull;
 
 import akka.actor.ActorRef;
 import static java.util.Objects.requireNonNull;
 
 import akka.actor.ActorRef;

-import com.google.common.base.Verify;

 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Collection;
 import java.util.Iterator;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Collection;
 import java.util.Iterator;


@@ -130,7 +131,7 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
 
         @Override
         void onTransactionCompleted(final AbstractProxyTransaction tx) {
 
         @Override
         void onTransactionCompleted(final AbstractProxyTransaction tx) {

-            Verify.verify(tx instanceof LocalProxyTransaction);
+            verify(tx instanceof LocalProxyTransaction, "Unexpected transaction %s", tx);

             if (tx instanceof LocalReadWriteProxyTransaction
                     && LAST_SEALED_UPDATER.compareAndSet(this, (LocalReadWriteProxyTransaction) tx, null)) {
                 LOG.debug("Completed last sealed transaction {}", tx);
             if (tx instanceof LocalReadWriteProxyTransaction
                     && LAST_SEALED_UPDATER.compareAndSet(this, (LocalReadWriteProxyTransaction) tx, null)) {
                 LOG.debug("Completed last sealed transaction {}", tx);


@@ -229,7 +230,7 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
                 final ConnectionEntry e = it.next();
                 final Request<?, ?> req = e.getRequest();
                 if (identifier.equals(req.getTarget())) {
                 final ConnectionEntry e = it.next();
                 final Request<?, ?> req = e.getRequest();
                 if (identifier.equals(req.getTarget())) {

-                    Verify.verify(req instanceof LocalHistoryRequest);
+                    verify(req instanceof LocalHistoryRequest, "Unexpected request %s", req);

                     if (req instanceof CreateLocalHistoryRequest) {
                         successor.connection.enqueueRequest(req, e.getCallback(), e.getEnqueuedTicks());
                         it.remove();
                     if (req instanceof CreateLocalHistoryRequest) {
                         successor.connection.enqueueRequest(req, e.getCallback(), e.getEnqueuedTicks());
                         it.remove();


@@ -249,7 +250,7 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
                 final ConnectionEntry e  = it.next();
                 final Request<?, ?> req = e.getRequest();
                 if (identifier.equals(req.getTarget())) {
                 final ConnectionEntry e  = it.next();
                 final Request<?, ?> req = e.getRequest();
                 if (identifier.equals(req.getTarget())) {

-                    Verify.verify(req instanceof LocalHistoryRequest);
+                    verify(req instanceof LocalHistoryRequest, "Unexpected request %s", req);

                     if (req instanceof DestroyLocalHistoryRequest) {
                         successor.connection.enqueueRequest(req, e.getCallback(), e.getEnqueuedTicks());
                         it.remove();
                     if (req instanceof DestroyLocalHistoryRequest) {
                         successor.connection.enqueueRequest(req, e.getCallback(), e.getEnqueuedTicks());
                         it.remove();


@@ -259,10 +260,10 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
             }
         }
 
             }
         }
 

-        @GuardedBy("lock")
+        @Holding("lock")

         @Override
         ProxyHistory finishReconnect() {
         @Override
         ProxyHistory finishReconnect() {

-            final ProxyHistory ret = Verify.verifyNotNull(successor);
+            final ProxyHistory ret = verifyNotNull(successor);

 
             for (AbstractProxyTransaction t : proxies.values()) {
                 t.finishReconnect();
 
             for (AbstractProxyTransaction t : proxies.values()) {
                 t.finishReconnect();


@@ -352,6 +353,7 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
     }
 
     @Override
     }
 
     @Override

+    // Non-final for mocking

     public LocalHistoryIdentifier getIdentifier() {
         return identifier;
     }
     public LocalHistoryIdentifier getIdentifier() {
         return identifier;
     }


@@ -377,6 +379,7 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
         return createTransactionProxy(txId, snapshotOnly, false);
     }
 
         return createTransactionProxy(txId, snapshotOnly, false);
     }
 

+    // Non-final for mocking

     AbstractProxyTransaction createTransactionProxy(final TransactionIdentifier txId, final boolean snapshotOnly,
             final boolean isDone) {
         lock.lock();
     AbstractProxyTransaction createTransactionProxy(final TransactionIdentifier txId, final boolean snapshotOnly,
             final boolean isDone) {
         lock.lock();


@@ -417,7 +420,7 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
         }
     }
 
         }
     }
 

-    void purgeTransaction(final AbstractProxyTransaction tx) {
+    final void purgeTransaction(final AbstractProxyTransaction tx) {

         lock.lock();
         try {
             proxies.remove(tx.getIdentifier());
         lock.lock();
         try {
             proxies.remove(tx.getIdentifier());


@@ -462,7 +465,7 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
     abstract ProxyHistory createSuccessor(AbstractClientConnection<ShardBackendInfo> connection);
 
     @SuppressFBWarnings(value = "UL_UNRELEASED_LOCK", justification = "Lock is released asynchronously via the cohort")
     abstract ProxyHistory createSuccessor(AbstractClientConnection<ShardBackendInfo> connection);
 
     @SuppressFBWarnings(value = "UL_UNRELEASED_LOCK", justification = "Lock is released asynchronously via the cohort")

-    ProxyReconnectCohort startReconnect(final ConnectedClientConnection<ShardBackendInfo> newConnection) {
+    final ProxyReconnectCohort startReconnect(final ConnectedClientConnection<ShardBackendInfo> newConnection) {

         lock.lock();
         if (successor != null) {
             lock.unlock();
         lock.lock();
         if (successor != null) {
             lock.unlock();


@@ -506,6 +509,7 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
         // No-op for most implementations
     }
 
         // No-op for most implementations
     }
 

+    @Holding("lock")

     void onTransactionSealed(final AbstractProxyTransaction tx) {
         // No-op on most implementations
     }
     void onTransactionSealed(final AbstractProxyTransaction tx) {
         // No-op on most implementations
     }

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java
index b102e8c45b3a956a2871b85ab8950cafc2294342..22536cc50af0199cda53a09b72b59f83da609eff 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java
@@ -77,7 +77,7 @@ abstract class AbstractFrontendHistory implements Identifiable<LocalHistoryIdent
     final @Nullable TransactionSuccess<?> handleTransactionRequest(final TransactionRequest<?> request,
             final RequestEnvelope envelope, final long now) throws RequestException {
         if (request instanceof TransactionPurgeRequest) {
     final @Nullable TransactionSuccess<?> handleTransactionRequest(final TransactionRequest<?> request,
             final RequestEnvelope envelope, final long now) throws RequestException {
         if (request instanceof TransactionPurgeRequest) {

-            return handleTransactionPurgeRequest(request, envelope, now);
+            return handleTransactionPurgeRequest((TransactionPurgeRequest) request, envelope, now);

         }
 
         final TransactionIdentifier id = request.getTarget();
         }
 
         final TransactionIdentifier id = request.getTarget();


@@ -117,7 +117,7 @@ abstract class AbstractFrontendHistory implements Identifiable<LocalHistoryIdent
         return tx.handleRequest(request, envelope, now);
     }
 
         return tx.handleRequest(request, envelope, now);
     }
 

-    private TransactionSuccess<?> handleTransactionPurgeRequest(final TransactionRequest<?> request,
+    private TransactionPurgeResponse handleTransactionPurgeRequest(final TransactionPurgeRequest request,

             final RequestEnvelope envelope, final long now) {
         final TransactionIdentifier id = request.getTarget();
         final long txidBits = id.getTransactionId();
             final RequestEnvelope envelope, final long now) {
         final TransactionIdentifier id = request.getTarget();
         final long txidBits = id.getTransactionId();


@@ -203,8 +203,7 @@ abstract class AbstractFrontendHistory implements Identifiable<LocalHistoryIdent
 
     abstract FrontendTransaction createOpenTransaction(TransactionIdentifier id);
 
 
     abstract FrontendTransaction createOpenTransaction(TransactionIdentifier id);
 

-    abstract FrontendTransaction createReadyTransaction(TransactionIdentifier id, DataTreeModification mod)
-        ;
+    abstract FrontendTransaction createReadyTransaction(TransactionIdentifier id, DataTreeModification mod);

 
     abstract ShardDataTreeCohort createFailedCohort(TransactionIdentifier id, DataTreeModification mod,
             Exception failure);
 
     abstract ShardDataTreeCohort createFailedCohort(TransactionIdentifier id, DataTreeModification mod,
             Exception failure);


@@ -213,8 +212,11 @@ abstract class AbstractFrontendHistory implements Identifiable<LocalHistoryIdent
             Optional<SortedSet<String>> participatingShardNames);
 
     @Override
             Optional<SortedSet<String>> participatingShardNames);
 
     @Override

-    public String toString() {
-        return MoreObjects.toStringHelper(this).omitNullValues().add("identifier", getIdentifier())
-                .add("persistenceId", persistenceId).add("transactions", transactions).toString();
+    public final String toString() {
+        return MoreObjects.toStringHelper(this).omitNullValues()
+            .add("identifier", getIdentifier())
+            .add("persistenceId", persistenceId)
+            .add("transactions", transactions)
+            .toString();

     }
 }
     }
 }

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java
index ba64bad55d36d627717fc6550f8f7772d719bd72..e086e51a66f706d103168bbe36db50841e3c68eb 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java
@@ -14,6 +14,7 @@ import com.google.common.base.MoreObjects.ToStringHelper;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;

+import org.eclipse.jdt.annotation.NonNull;

 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.controller.cluster.access.commands.CreateLocalHistoryRequest;
 import org.opendaylight.controller.cluster.access.commands.DeadHistoryException;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.controller.cluster.access.commands.CreateLocalHistoryRequest;
 import org.opendaylight.controller.cluster.access.commands.DeadHistoryException;


@@ -99,7 +100,7 @@ abstract class LeaderFrontendState implements Identifiable<ClientIdentifier> {
                 } else if (request instanceof DestroyLocalHistoryRequest) {
                     return handleDestroyHistory((DestroyLocalHistoryRequest) request, envelope, now);
                 } else if (request instanceof PurgeLocalHistoryRequest) {
                 } else if (request instanceof DestroyLocalHistoryRequest) {
                     return handleDestroyHistory((DestroyLocalHistoryRequest) request, envelope, now);
                 } else if (request instanceof PurgeLocalHistoryRequest) {

-                    return handlePurgeHistory((PurgeLocalHistoryRequest)request, envelope, now);
+                    return handlePurgeHistory((PurgeLocalHistoryRequest) request, envelope, now);

                 } else {
                     LOG.warn("{}: rejecting unsupported request {}", persistenceId(), request);
                     throw new UnsupportedRequestException(request);
                 } else {
                     LOG.warn("{}: rejecting unsupported request {}", persistenceId(), request);
                     throw new UnsupportedRequestException(request);


@@ -236,9 +237,9 @@ abstract class LeaderFrontendState implements Identifiable<ClientIdentifier> {
 
     private static final Logger LOG = LoggerFactory.getLogger(LeaderFrontendState.class);
 
 
     private static final Logger LOG = LoggerFactory.getLogger(LeaderFrontendState.class);
 

-    private final ShardDataTree tree;
-    private final ClientIdentifier clientId;
-    private final String persistenceId;
+    private final @NonNull ClientIdentifier clientId;
+    private final @NonNull String persistenceId;
+    private final @NonNull ShardDataTree tree;

 
     private long lastConnectTicks;
     private long lastSeenTicks;
 
     private long lastConnectTicks;
     private long lastSeenTicks;