From 9a5a654be068a0793c7734b2a8a57470cf8aa7cf Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sun, 11 Feb 2024 18:35:31 +0100 Subject: [PATCH] Move TransactionChainManagerTest Tests should really be co-located with what they are testing. Move TransactionChainManagerTest to openflowplugin-common. Change-Id: Id3e89709431001148236d339f8e5de3e4a6d67e8 Signed-off-by: Robert Varga --- openflowplugin-common/pom.xml | 4 +- .../txchain/TransactionChainManager.java | 64 +++++++++---------- .../txchain}/TransactionChainManagerTest.java | 9 +-- 3 files changed, 34 insertions(+), 43 deletions(-) rename {openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device => openflowplugin-common/src/test/java/org/opendaylight/openflowplugin/common/txchain}/TransactionChainManagerTest.java (95%) diff --git a/openflowplugin-common/pom.xml b/openflowplugin-common/pom.xml index 8ea474cf1a..19a77f6603 100644 --- a/openflowplugin-common/pom.xml +++ b/openflowplugin-common/pom.xml @@ -23,8 +23,8 @@ - org.slf4j - slf4j-simple + org.opendaylight.openflowplugin.model + model-inventory test diff --git a/openflowplugin-common/src/main/java/org/opendaylight/openflowplugin/common/txchain/TransactionChainManager.java b/openflowplugin-common/src/main/java/org/opendaylight/openflowplugin/common/txchain/TransactionChainManager.java index 2b18d3ba38..ba3fcda84f 100755 --- a/openflowplugin-common/src/main/java/org/opendaylight/openflowplugin/common/txchain/TransactionChainManager.java +++ b/openflowplugin-common/src/main/java/org/opendaylight/openflowplugin/common/txchain/TransactionChainManager.java @@ -7,6 +7,8 @@ */ package org.opendaylight.openflowplugin.common.txchain; +import static java.util.Objects.requireNonNull; + import com.google.common.base.Preconditions; import com.google.common.util.concurrent.FluentFuture; import com.google.common.util.concurrent.FutureCallback; @@ -20,7 +22,6 @@ import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import org.checkerframework.checker.lock.qual.GuardedBy; import org.checkerframework.checker.lock.qual.Holding; -import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.mdsal.binding.api.DataBroker; import org.opendaylight.mdsal.binding.api.ReadWriteTransaction; import org.opendaylight.mdsal.binding.api.Transaction; @@ -48,6 +49,7 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl private static final Logger LOG = LoggerFactory.getLogger(TransactionChainManager.class); private static final String CANNOT_WRITE_INTO_TRANSACTION = "Cannot write into transaction."; + private final ReadWriteLock readWriteTransactionLock = new ReentrantReadWriteLock(); private final Object txLock = new Object(); private final DataBroker dataBroker; private final String nodeId; @@ -59,19 +61,15 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl @GuardedBy("txLock") private boolean submitIsEnabled; @GuardedBy("txLock") - private FluentFuture lastSubmittedFuture; - - private volatile boolean initCommit; - + private FluentFuture lastSubmittedFuture = CommitInfo.emptyFluentFuture(); @GuardedBy("txLock") private TransactionChainManagerStatus transactionChainManagerStatus = TransactionChainManagerStatus.SLEEPING; - private ReadWriteLock readWriteTransactionLock = new ReentrantReadWriteLock(); - public TransactionChainManager(@NonNull final DataBroker dataBroker, - @NonNull final String deviceIdentifier) { - this.dataBroker = dataBroker; - this.nodeId = deviceIdentifier; - this.lastSubmittedFuture = CommitInfo.emptyFluentFuture(); + private volatile boolean initCommit; + + public TransactionChainManager(final DataBroker dataBroker, final String nodeId) { + this.dataBroker = requireNonNull(dataBroker); + this.nodeId = requireNonNull(nodeId); } @Holding("txLock") @@ -96,7 +94,7 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl public void activateTransactionManager() { if (LOG.isDebugEnabled()) { LOG.debug("activateTransactionManager for node {} transaction submit is set to {}", - this.nodeId, submitIsEnabled); + nodeId, submitIsEnabled); } synchronized (txLock) { if (TransactionChainManagerStatus.SLEEPING == transactionChainManagerStatus) { @@ -104,9 +102,9 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl "TxChainFactory survive last close."); Preconditions.checkState(writeTx == null, "We have some unexpected WriteTransaction."); - this.transactionChainManagerStatus = TransactionChainManagerStatus.WORKING; - this.submitIsEnabled = false; - this.initCommit = true; + transactionChainManagerStatus = TransactionChainManagerStatus.WORKING; + submitIsEnabled = false; + initCommit = true; createTxChain(); } } @@ -120,7 +118,7 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl */ public FluentFuture deactivateTransactionManager() { if (LOG.isDebugEnabled()) { - LOG.debug("deactivateTransactionManager for node {}", this.nodeId); + LOG.debug("deactivateTransactionManager for node {}", nodeId); } final FluentFuture future; synchronized (txLock) { @@ -136,7 +134,7 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl } @Override - public void onFailure(@NonNull final Throwable throwable) { + public void onFailure(final Throwable throwable) { closeTransactionChain(); } }, MoreExecutors.directExecutor()); @@ -166,7 +164,7 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl @GuardedBy("txLock") @SuppressWarnings("checkstyle:IllegalCatch") - public boolean submitTransaction(boolean doSync) { + public boolean submitTransaction(final boolean doSync) { synchronized (txLock) { if (!submitIsEnabled) { LOG.trace("transaction not committed - submit block issued"); @@ -178,7 +176,7 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl } Preconditions.checkState(TransactionChainManagerStatus.WORKING == transactionChainManagerStatus, "we have here Uncompleted Transaction for node {} and we are not MASTER", - this.nodeId); + nodeId); final FluentFuture submitFuture = writeTx.commit(); lastSubmittedFuture = submitFuture; writeTx = null; @@ -206,13 +204,11 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl public void onFailure(final Throwable throwable) { if (throwable instanceof InterruptedException || throwable instanceof ExecutionException) { LOG.error("Transaction commit failed. ", throwable); + } else if (throwable instanceof CancellationException) { + LOG.warn("Submit task was canceled"); + LOG.trace("Submit exception: ", throwable); } else { - if (throwable instanceof CancellationException) { - LOG.warn("Submit task was canceled"); - LOG.trace("Submit exception: ", throwable); - } else { - LOG.error("Exception during transaction submitting. ", throwable); - } + LOG.error("Exception during transaction submitting. ", throwable); } } }, MoreExecutors.directExecutor()); @@ -225,7 +221,7 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl synchronized (txLock) { ensureTransaction(); if (writeTx == null) { - LOG.debug("WriteTx is null for node {}. Delete {} was not realized.", this.nodeId, path); + LOG.debug("WriteTx is null for node {}. Delete {} was not realized.", nodeId, path); throw new TransactionChainClosedException(CANNOT_WRITE_INTO_TRANSACTION); } @@ -240,7 +236,7 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl synchronized (txLock) { ensureTransaction(); if (writeTx == null) { - LOG.debug("WriteTx is null for node {}. Write data for {} was not realized.", this.nodeId, path); + LOG.debug("WriteTx is null for node {}. Write data for {} was not realized.", nodeId, path); throw new TransactionChainClosedException(CANNOT_WRITE_INTO_TRANSACTION); } @@ -259,7 +255,7 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl synchronized (txLock) { ensureTransaction(); if (writeTx == null) { - LOG.debug("WriteTx is null for node {}. Merge data for {} was not realized.", this.nodeId, path); + LOG.debug("WriteTx is null for node {}. Merge data for {} was not realized.", nodeId, path); throw new TransactionChainClosedException(CANNOT_WRITE_INTO_TRANSACTION); } @@ -276,7 +272,7 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl synchronized (txLock) { ensureTransaction(); if (writeTx == null) { - LOG.debug("WriteTx is null for node {}. Read data for {} was not realized.", this.nodeId, path); + LOG.debug("WriteTx is null for node {}. Read data for {} was not realized.", nodeId, path); throw new TransactionChainClosedException(CANNOT_WRITE_INTO_TRANSACTION); } @@ -289,7 +285,7 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl final Transaction transaction, final Throwable cause) { synchronized (txLock) { if (TransactionChainManagerStatus.WORKING == transactionChainManagerStatus - && chain.equals(this.transactionChain)) { + && chain.equals(transactionChain)) { LOG.warn("Transaction chain failed, recreating chain due to ", cause); closeTransactionChain(); createTxChain(); @@ -320,10 +316,10 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl public FluentFuture shuttingDown() { if (LOG.isDebugEnabled()) { - LOG.debug("TxManager is going SHUTTING_DOWN for node {}", this.nodeId); + LOG.debug("TxManager is going SHUTTING_DOWN for node {}", nodeId); } synchronized (txLock) { - this.transactionChainManagerStatus = TransactionChainManagerStatus.SHUTTING_DOWN; + transactionChainManagerStatus = TransactionChainManagerStatus.SHUTTING_DOWN; return txChainShuttingDown(); } } @@ -347,7 +343,7 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl future = lastSubmittedFuture; } else { if (LOG.isDebugEnabled()) { - LOG.debug("Submitting all transactions for Node {}", this.nodeId); + LOG.debug("Submitting all transactions for Node {}", nodeId); } // hijack md-sal thread future = writeTx.commit(); @@ -360,7 +356,7 @@ public class TransactionChainManager implements TransactionChainListener, AutoCl @Override public void close() { if (LOG.isDebugEnabled()) { - LOG.debug("Setting transactionChainManagerStatus to SHUTTING_DOWN for {}", this.nodeId); + LOG.debug("Setting transactionChainManagerStatus to SHUTTING_DOWN for {}", nodeId); } synchronized (txLock) { closeTransactionChain(); diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/TransactionChainManagerTest.java b/openflowplugin-common/src/test/java/org/opendaylight/openflowplugin/common/txchain/TransactionChainManagerTest.java similarity index 95% rename from openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/TransactionChainManagerTest.java rename to openflowplugin-common/src/test/java/org/opendaylight/openflowplugin/common/txchain/TransactionChainManagerTest.java index d6d77acab5..776813c9a2 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/TransactionChainManagerTest.java +++ b/openflowplugin-common/src/test/java/org/opendaylight/openflowplugin/common/txchain/TransactionChainManagerTest.java @@ -5,7 +5,7 @@ * 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.openflowplugin.impl.device; +package org.opendaylight.openflowplugin.common.txchain; import static org.mockito.ArgumentMatchers.any; @@ -25,9 +25,6 @@ import org.opendaylight.mdsal.binding.api.TransactionChain; import org.opendaylight.mdsal.binding.api.TransactionChainListener; import org.opendaylight.mdsal.common.api.CommitInfo; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; -import org.opendaylight.openflowplugin.api.openflow.device.DeviceInfo; -import org.opendaylight.openflowplugin.common.txchain.TransactionChainManager; -import org.opendaylight.openflowplugin.impl.util.DeviceStateUtil; import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeId; import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.Nodes; import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node; @@ -48,8 +45,6 @@ public class TransactionChainManagerTest { private ReadWriteTransaction writeTx; @Mock private TransactionChain transactionChain; - @Mock - DeviceInfo deviceInfo; @Mock private KeyedInstanceIdentifier nodeKeyIdent; @@ -64,7 +59,7 @@ public class TransactionChainManagerTest { Mockito.when(dataBroker.createTransactionChain(any(TransactionChainListener.class))) .thenReturn(txChain); nodeId = new NodeId("h2g2:42"); - nodeKeyIdent = DeviceStateUtil.createNodeInstanceIdentifier(nodeId); + nodeKeyIdent = InstanceIdentifier.create(Nodes.class).child(Node.class, new NodeKey(nodeId)); txChainManager = new TransactionChainManager(dataBroker, nodeId.getValue()); Mockito.when(txChain.newReadWriteTransaction()).thenReturn(writeTx); -- 2.36.6