From 3990d382057f94251190a78295b4aa47f2fe7112 Mon Sep 17 00:00:00 2001 From: Andrej Leitner Date: Mon, 8 Aug 2016 11:39:02 +0200 Subject: [PATCH 1/1] Sonar - technical debt of FRS app Change-Id: I2951c3e85f3599aec951844289de22e25b652a36 Signed-off-by: Andrej Leitner --- .../frsync/impl/SimplifiedConfigListener.java | 24 +++--- .../impl/SimplifiedOperationalListener.java | 33 ++++---- .../impl/SyncReactorGuardDecorator.java | 59 +++++++------- .../frsync/impl/SyncReactorImpl.java | 22 ++--- .../clustering/DeviceMastershipManager.java | 2 +- .../frsync/impl/strategy/FlowForwarder.java | 7 +- .../SyncPlanPushStrategyFlatBatchImpl.java | 81 +++++++++++-------- .../applications/frsync/util/FxChainUtil.java | 4 + .../frsync/util/ModificationUtil.java | 5 ++ .../applications/frsync/util/PathUtil.java | 4 + .../frsync/util/ReconcileUtil.java | 74 ++++++++--------- .../frsync/util/SwitchFlowId.java | 46 +++++------ .../applications/frsync/util/SyncupEntry.java | 10 +++ .../impl/ForwardingRulesSyncProviderTest.java | 7 ++ 14 files changed, 190 insertions(+), 188 deletions(-) diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedConfigListener.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedConfigListener.java index f4188e2926..6f86def8a6 100644 --- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedConfigListener.java +++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedConfigListener.java @@ -26,7 +26,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Listens to config changes and delegates add/remove/update/barrier to {@link SyncReactor}. + * Listens to config changes and delegates sync entry to {@link SyncReactor}. */ public class SimplifiedConfigListener extends AbstractFrmSyncListener { private static final Logger LOG = LoggerFactory.getLogger(SimplifiedConfigListener.class); @@ -49,8 +49,8 @@ public class SimplifiedConfigListener extends AbstractFrmSyncListener> processNodeModification( @@ -82,14 +82,9 @@ public class SimplifiedConfigListener extends AbstractFrmSyncListener - *
  • Note very first time after restart was handled by operational listener. Syncup should - * calculate no delta (we don want to reconfigure switch if not necessary).
    • - *
  • But later the config. node could be deleted, after that config node added again. Syncup - * should calculate that everything needs to be added. Operational store should be empty in - * optimal case (but the switch could be reprogrammed by another person/system.
    • - * + * Add only what is missing on device. If node was added to config DS and it is already present + * in operational DS (connected) diff between current new configuration and actual configuration + * (seen in operational) should be calculated and sent to device. */ private ListenableFuture onNodeAdded(final InstanceIdentifier nodePath, final FlowCapableNode dataAfter, @@ -102,7 +97,7 @@ public class SimplifiedConfigListener extends AbstractFrmSyncListener onNodeDeleted(final InstanceIdentifier nodePath, final FlowCapableNode dataBefore) throws InterruptedException { diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListener.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListener.java index eb70f5d493..2f0fc01257 100644 --- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListener.java +++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListener.java @@ -40,7 +40,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Listens to operational new nodes and delegates add/remove/update/barrier to {@link SyncReactor}. + * Listens to operational changes and starts reconciliation through {@link SyncReactor} when necessary. */ public class SimplifiedOperationalListener extends AbstractFrmSyncListener { private static final Logger LOG = LoggerFactory.getLogger(SimplifiedOperationalListener.class); @@ -71,13 +71,8 @@ public class SimplifiedOperationalListener extends AbstractFrmSyncListener } /** - * This method behaves like this: - *
      - *
    • If node is added to operational store then reconciliation.
      • - *
    • Node is deleted from operational cache is removed.
      • - *
    • Skip this event otherwise.
      • - *
    - * + * Update cache, register for device masterhip when device connected and start reconciliation if device + * is registered and actual modification is consistent.Skip the event otherwise. * @throws InterruptedException from syncup */ protected Optional> processNodeModification( @@ -89,7 +84,7 @@ public class SimplifiedOperationalListener extends AbstractFrmSyncListener deviceMastershipManager.onDeviceConnected(nodeId); } - if (isRegisteredAndConsistentForReconcile(modification)) { + if (reconciliationRegistry.isRegistered(nodeId) && isConsistentForReconcile(modification)) { return reconciliation(modification); } else { return skipModification(modification); @@ -98,7 +93,7 @@ public class SimplifiedOperationalListener extends AbstractFrmSyncListener /** * Remove if delete. Update only if FlowCapableNode Augmentation modified. - * + * Unregister for device mastership. * @param modification Datastore modification */ private void updateCache(DataTreeModification modification) { @@ -169,6 +164,13 @@ public class SimplifiedOperationalListener extends AbstractFrmSyncListener return false; } + /** + * If node is present in config DS diff between wanted configuration (in config DS) and actual device + * configuration (coming from operational) should be calculated and sent to device. + * @param modification from DS + * @return optional syncup future + * @throws InterruptedException from syncup + */ private Optional> reconciliation(DataTreeModification modification) throws InterruptedException { final NodeId nodeId = ModificationUtil.nodeId(modification); final Optional nodeConfiguration = configDao.loadByNodeId(nodeId); @@ -188,13 +190,8 @@ public class SimplifiedOperationalListener extends AbstractFrmSyncListener } } - private boolean isRegisteredAndConsistentForReconcile(DataTreeModification modification) { + private boolean isConsistentForReconcile(DataTreeModification modification) { final NodeId nodeId = PathUtil.digNodeId(modification.getRootPath().getRootIdentifier()); - - if (!reconciliationRegistry.isRegistered(nodeId)) { - return false; - } - final FlowCapableStatisticsGatheringStatus gatheringStatus = modification.getRootNode().getDataAfter() .getAugmentation(FlowCapableStatisticsGatheringStatus.class); @@ -234,9 +231,7 @@ public class SimplifiedOperationalListener extends AbstractFrmSyncListener if (node == null) { return true; } - final List nodeConnectors = node.getNodeConnector(); - return nodeConnectors == null || nodeConnectors.isEmpty(); } @@ -244,5 +239,5 @@ public class SimplifiedOperationalListener extends AbstractFrmSyncListener public LogicalDatastoreType dsType() { return LogicalDatastoreType.OPERATIONAL; } - + } diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SyncReactorGuardDecorator.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SyncReactorGuardDecorator.java index c51f0bb6e0..3f1b2716a3 100644 --- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SyncReactorGuardDecorator.java +++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SyncReactorGuardDecorator.java @@ -63,35 +63,7 @@ public class SyncReactorGuardDecorator implements SyncReactor { final ListenableFuture endResult = delegate.syncup(flowcapableNodePath, syncupEntry); - Futures.addCallback(endResult, new FutureCallback() { - @Override - public void onSuccess(@Nullable final Boolean result) { - if (LOG.isDebugEnabled()) { - final long stampFinished = System.nanoTime(); - LOG.debug("syncup finished {} took:{} rpc:{} wait:{} guard:{} permits thread:{}", nodeId.getValue(), - formatNanos(stampFinished - stampBeforeGuard), - formatNanos(stampFinished - stampAfterGuard), - formatNanos(stampAfterGuard - stampBeforeGuard), - guard.availablePermits(), threadName()); - } - - releaseGuardForNodeId(guard); - } - - @Override - public void onFailure(final Throwable t) { - if (LOG.isDebugEnabled()) { - final long stampFinished = System.nanoTime(); - LOG.warn("syncup failed {} took:{} rpc:{} wait:{} guard:{} permits thread:{}", nodeId.getValue(), - formatNanos(stampFinished - stampBeforeGuard), - formatNanos(stampFinished - stampAfterGuard), - formatNanos(stampAfterGuard - stampBeforeGuard), - guard.availablePermits(), threadName()); - } - - releaseGuardForNodeId(guard); - } - }); + Futures.addCallback(endResult, createSyncupCallback(guard, stampBeforeGuard, stampAfterGuard, nodeId)); return endResult; } catch (InterruptedException e) { releaseGuardForNodeId(guard); @@ -99,7 +71,32 @@ public class SyncReactorGuardDecorator implements SyncReactor { } } - private String formatNanos(long nanos) { + private static FutureCallback createSyncupCallback(final Semaphore guard, + final long stampBeforeGuard, + final long stampAfterGuard, + final NodeId nodeId) { + return new FutureCallback() { + @Override + public void onSuccess(@Nullable final Boolean result) { + if (LOG.isDebugEnabled()) { + final long stampFinished = System.nanoTime(); + LOG.debug("syncup finished {} took:{} rpc:{} wait:{} guard:{} permits thread:{}", nodeId.getValue(), + formatNanos(stampFinished - stampBeforeGuard), formatNanos(stampFinished - stampAfterGuard), + formatNanos(stampAfterGuard - stampBeforeGuard), guard.availablePermits(), threadName()); + } + releaseGuardForNodeId(guard); + } + @Override + public void onFailure(final Throwable t) { + final long stampFinished = System.nanoTime(); + LOG.error("syncup failed {} took:{} rpc:{} wait:{} guard:{} permits thread:{}", nodeId.getValue(), + formatNanos(stampFinished - stampBeforeGuard), formatNanos(stampFinished - stampAfterGuard), + formatNanos(stampAfterGuard - stampBeforeGuard), guard.availablePermits(), threadName()); + releaseGuardForNodeId(guard); + }}; + } + + private static String formatNanos(long nanos) { return "'" + TimeUnit.NANOSECONDS.toMillis(nanos) + " ms'"; } @@ -126,7 +123,7 @@ public class SyncReactorGuardDecorator implements SyncReactor { * Unlock and release guard. * @param guard semaphore guard which should be unlocked */ - private void releaseGuardForNodeId(final Semaphore guard) { + private static void releaseGuardForNodeId(final Semaphore guard) { if (guard != null) { guard.release(); LOG.trace("syncup release guard:{} thread:{}", guard, threadName()); diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SyncReactorImpl.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SyncReactorImpl.java index 5eb8860f88..7f1be0b3fd 100644 --- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SyncReactorImpl.java +++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SyncReactorImpl.java @@ -107,30 +107,20 @@ public class SyncReactorImpl implements SyncReactor { if (input == null) { return false; } - if (LOG.isDebugEnabled()) { final CrudCounts flowCrudCounts = counters.getFlowCrudCounts(); final CrudCounts meterCrudCounts = counters.getMeterCrudCounts(); final CrudCounts groupCrudCounts = counters.getGroupCrudCounts(); - LOG.debug("syncup outcome[{}] (added/updated/removed): flow={}/{}/{}, meter={}/{}/{}, group={}/{}/{}, took={} ms", + LOG.debug("syncup outcome[{}] (added/updated/removed): flow={}/{}/{}, group={}/{}/{}, meter={}/{}/{}, took={} ms", nodeId.getValue(), - flowCrudCounts.getAdded(), - flowCrudCounts.getUpdated(), - flowCrudCounts.getRemoved(), - meterCrudCounts.getAdded(), - meterCrudCounts.getUpdated(), - meterCrudCounts.getRemoved(), - groupCrudCounts.getAdded(), - groupCrudCounts.getUpdated(), - groupCrudCounts.getRemoved(), - TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - counters.getStartNano()) - ); + flowCrudCounts.getAdded(), flowCrudCounts.getUpdated(), flowCrudCounts.getRemoved(), + groupCrudCounts.getAdded(), groupCrudCounts.getUpdated(), groupCrudCounts.getRemoved(), + meterCrudCounts.getAdded(), meterCrudCounts.getUpdated(), meterCrudCounts.getRemoved(), + TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - counters.getStartNano())); } - LOG.trace("syncup errors: {}", input.getErrors()); return input.isSuccessful(); - } - }); + }}); } @VisibleForTesting diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/clustering/DeviceMastershipManager.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/clustering/DeviceMastershipManager.java index 41f736c05d..06532ad3c2 100644 --- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/clustering/DeviceMastershipManager.java +++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/clustering/DeviceMastershipManager.java @@ -48,7 +48,7 @@ public class DeviceMastershipManager { try { registration.close(); } catch (Exception e) { - LOG.error("FRS cluster service close fail: {}", nodeId.getValue()); + LOG.error("FRS cluster service close fail: {} {}", nodeId.getValue(), e); } } LOG.debug("FRS service unregistered for: {}", nodeId.getValue()); diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/strategy/FlowForwarder.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/strategy/FlowForwarder.java index ca752470fe..2609d576f7 100644 --- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/strategy/FlowForwarder.java +++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/strategy/FlowForwarder.java @@ -41,6 +41,7 @@ import org.slf4j.LoggerFactory; public class FlowForwarder implements ForwardingRulesCommitter { private static final Logger LOG = LoggerFactory.getLogger(FlowForwarder.class); + private static final String TABLE_ID_MISMATCH = "tableId mismatch"; private final SalFlowService salFlowService; public FlowForwarder(final SalFlowService salFlowService) { @@ -67,7 +68,7 @@ public class FlowForwarder implements ForwardingRulesCommitterfailed() - .withError(RpcError.ErrorType.APPLICATION, "tableId mismatch").buildFuture(); + .withError(RpcError.ErrorType.APPLICATION, TABLE_ID_MISMATCH).buildFuture(); } } @@ -94,7 +95,7 @@ public class FlowForwarder implements ForwardingRulesCommitterfailed() - .withError(RpcError.ErrorType.APPLICATION, "tableId mismatch").buildFuture(); + .withError(RpcError.ErrorType.APPLICATION, TABLE_ID_MISMATCH).buildFuture(); } return output; @@ -117,7 +118,7 @@ public class FlowForwarder implements ForwardingRulesCommitterfailed().withError(RpcError.ErrorType.APPLICATION, "tableId mismatch").buildFuture(); + output = RpcResultBuilder.failed().withError(RpcError.ErrorType.APPLICATION, TABLE_ID_MISMATCH).buildFuture(); } return output; } diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/strategy/SyncPlanPushStrategyFlatBatchImpl.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/strategy/SyncPlanPushStrategyFlatBatchImpl.java index 70ce219708..68e0803723 100644 --- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/strategy/SyncPlanPushStrategyFlatBatchImpl.java +++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/strategy/SyncPlanPushStrategyFlatBatchImpl.java @@ -174,17 +174,7 @@ public class SyncPlanPushStrategyFlatBatchImpl implements SyncPlanPushStrategy { public void onSuccess(@Nullable final RpcResult result) { if (!result.isSuccessful() && result.getResult() != null && !result.getResult().getBatchFailure().isEmpty()) { Map, Batch> batchMap = mapBatchesToRanges(inputBatchBag, failureIndexLimit); - - for (BatchFailure batchFailure : result.getResult().getBatchFailure()) { - for (Map.Entry, Batch> rangeBatchEntry : batchMap.entrySet()) { - if (rangeBatchEntry.getKey().contains(batchFailure.getBatchOrder())) { - // get type and decrease - final BatchChoice batchChoice = rangeBatchEntry.getValue().getBatchChoice(); - decrementCounters(batchChoice, counters); - break; - } - } - } + decrementBatchFailuresCounters(result.getResult().getBatchFailure(), batchMap, counters); } } @@ -195,6 +185,21 @@ public class SyncPlanPushStrategyFlatBatchImpl implements SyncPlanPushStrategy { }; } + private static void decrementBatchFailuresCounters(final List batchFailures, + final Map, Batch> batchMap, + final SyncCrudCounters counters) { + for (BatchFailure batchFailure : batchFailures) { + for (Map.Entry, Batch> rangeBatchEntry : batchMap.entrySet()) { + if (rangeBatchEntry.getKey().contains(batchFailure.getBatchOrder())) { + // get type and decrease + final BatchChoice batchChoice = rangeBatchEntry.getValue().getBatchChoice(); + decrementCounters(batchChoice, counters); + break; + } + } + } + } + static void decrementCounters(final BatchChoice batchChoice, final SyncCrudCounters counters) { if (batchChoice instanceof FlatBatchAddFlowCase) { counters.getFlowCrudCounts().decAdded(); @@ -233,6 +238,7 @@ public class SyncPlanPushStrategyFlatBatchImpl implements SyncPlanPushStrategy { @VisibleForTesting static int assembleRemoveFlows(final List batchBag, int batchOrder, final Map> flowItemSyncTableMap) { // process flow remove + int order = batchOrder; if (flowItemSyncTableMap != null) { for (Map.Entry> syncBoxEntry : flowItemSyncTableMap.entrySet()) { final ItemSyncBox flowItemSyncBox = syncBoxEntry.getValue(); @@ -251,19 +257,20 @@ public class SyncPlanPushStrategyFlatBatchImpl implements SyncPlanPushStrategy { .setBatchChoice(new FlatBatchRemoveFlowCaseBuilder() .setFlatBatchRemoveFlow(flatBatchRemoveFlowBag) .build()) - .setBatchOrder(batchOrder) + .setBatchOrder(order) .build(); - batchOrder += itemOrder; + order += itemOrder; batchBag.add(batch); } } } - return batchOrder; + return order; } @VisibleForTesting static int assembleAddOrUpdateGroups(final List batchBag, int batchOrder, final List> groupsToAddOrUpdate) { // process group add+update + int order = batchOrder; if (groupsToAddOrUpdate != null) { for (ItemSyncBox groupItemSyncBox : groupsToAddOrUpdate) { if (!groupItemSyncBox.getItemsToPush().isEmpty()) { @@ -277,9 +284,9 @@ public class SyncPlanPushStrategyFlatBatchImpl implements SyncPlanPushStrategy { .setBatchChoice(new FlatBatchAddGroupCaseBuilder() .setFlatBatchAddGroup(flatBatchAddGroupBag) .build()) - .setBatchOrder(batchOrder) + .setBatchOrder(order) .build(); - batchOrder += itemOrder; + order += itemOrder; batchBag.add(batch); } @@ -298,19 +305,20 @@ public class SyncPlanPushStrategyFlatBatchImpl implements SyncPlanPushStrategy { .setBatchChoice(new FlatBatchUpdateGroupCaseBuilder() .setFlatBatchUpdateGroup(flatBatchUpdateGroupBag) .build()) - .setBatchOrder(batchOrder) + .setBatchOrder(order) .build(); - batchOrder += itemOrder; + order += itemOrder; batchBag.add(batch); } } } - return batchOrder; + return order; } @VisibleForTesting static int assembleRemoveGroups(final List batchBag, int batchOrder, final List> groupsToRemoveOrUpdate) { // process group add+update + int order = batchOrder; if (groupsToRemoveOrUpdate != null) { for (ItemSyncBox groupItemSyncBox : groupsToRemoveOrUpdate) { if (!groupItemSyncBox.getItemsToPush().isEmpty()) { @@ -324,19 +332,20 @@ public class SyncPlanPushStrategyFlatBatchImpl implements SyncPlanPushStrategy { .setBatchChoice(new FlatBatchRemoveGroupCaseBuilder() .setFlatBatchRemoveGroup(flatBatchRemoveGroupBag) .build()) - .setBatchOrder(batchOrder) + .setBatchOrder(order) .build(); - batchOrder += itemOrder; + order += itemOrder; batchBag.add(batch); } } } - return batchOrder; + return order; } @VisibleForTesting static int assembleAddOrUpdateMeters(final List batchBag, int batchOrder, final ItemSyncBox meterItemSyncBox) { // process meter add+update + int order = batchOrder; if (meterItemSyncBox != null) { if (!meterItemSyncBox.getItemsToPush().isEmpty()) { final List flatBatchAddMeterBag = @@ -349,9 +358,9 @@ public class SyncPlanPushStrategyFlatBatchImpl implements SyncPlanPushStrategy { .setBatchChoice(new FlatBatchAddMeterCaseBuilder() .setFlatBatchAddMeter(flatBatchAddMeterBag) .build()) - .setBatchOrder(batchOrder) + .setBatchOrder(order) .build(); - batchOrder += itemOrder; + order += itemOrder; batchBag.add(batch); } @@ -370,18 +379,19 @@ public class SyncPlanPushStrategyFlatBatchImpl implements SyncPlanPushStrategy { .setBatchChoice(new FlatBatchUpdateMeterCaseBuilder() .setFlatBatchUpdateMeter(flatBatchUpdateMeterBag) .build()) - .setBatchOrder(batchOrder) + .setBatchOrder(order) .build(); - batchOrder += itemOrder; + order += itemOrder; batchBag.add(batch); } } - return batchOrder; + return order; } @VisibleForTesting static int assembleRemoveMeters(final List batchBag, int batchOrder, final ItemSyncBox meterItemSyncBox) { // process meter remove + int order = batchOrder; if (meterItemSyncBox != null && !meterItemSyncBox.getItemsToPush().isEmpty()) { final List flatBatchRemoveMeterBag = new ArrayList<>(meterItemSyncBox.getItemsToUpdate().size()); @@ -393,17 +403,18 @@ public class SyncPlanPushStrategyFlatBatchImpl implements SyncPlanPushStrategy { .setBatchChoice(new FlatBatchRemoveMeterCaseBuilder() .setFlatBatchRemoveMeter(flatBatchRemoveMeterBag) .build()) - .setBatchOrder(batchOrder) + .setBatchOrder(order) .build(); - batchOrder += itemOrder; + order += itemOrder; batchBag.add(batch); } - return batchOrder; + return order; } @VisibleForTesting static int assembleAddOrUpdateFlows(final List batchBag, int batchOrder, final Map> flowItemSyncTableMap) { // process flow add+update + int order = batchOrder; if (flowItemSyncTableMap != null) { for (Map.Entry> syncBoxEntry : flowItemSyncTableMap.entrySet()) { final ItemSyncBox flowItemSyncBox = syncBoxEntry.getValue(); @@ -422,9 +433,9 @@ public class SyncPlanPushStrategyFlatBatchImpl implements SyncPlanPushStrategy { .setBatchChoice(new FlatBatchAddFlowCaseBuilder() .setFlatBatchAddFlow(flatBatchAddFlowBag) .build()) - .setBatchOrder(batchOrder) + .setBatchOrder(order) .build(); - batchOrder += itemOrder; + order += itemOrder; batchBag.add(batch); } @@ -444,14 +455,14 @@ public class SyncPlanPushStrategyFlatBatchImpl implements SyncPlanPushStrategy { .setBatchChoice(new FlatBatchUpdateFlowCaseBuilder() .setFlatBatchUpdateFlow(flatBatchUpdateFlowBag) .build()) - .setBatchOrder(batchOrder) + .setBatchOrder(order) .build(); - batchOrder += itemOrder; + order += itemOrder; batchBag.add(batch); } } } - return batchOrder; + return order; } public SyncPlanPushStrategyFlatBatchImpl setFlatBatchService(final SalFlatBatchService flatBatchService) { diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/FxChainUtil.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/FxChainUtil.java index 903dd207f0..5e02d48a39 100644 --- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/FxChainUtil.java +++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/FxChainUtil.java @@ -27,6 +27,10 @@ public class FxChainUtil { private static final Logger LOG = LoggerFactory.getLogger(FxChainUtil.class); + private FxChainUtil() { + throw new IllegalStateException("This class should not be instantiated."); + } + public static FutureCallback> logResultCallback(final NodeId nodeId, final String prefix) { return new FutureCallback>() { diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/ModificationUtil.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/ModificationUtil.java index 1df1f7c6a4..0eb7252725 100644 --- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/ModificationUtil.java +++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/ModificationUtil.java @@ -18,6 +18,11 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.N * Basic {@link DataTreeModification} related tools. */ public class ModificationUtil { + + private ModificationUtil() { + throw new IllegalStateException("This class should not be instantiated."); + } + public static String nodeIdValue(DataTreeModification modification) { final NodeId nodeId = nodeId(modification); diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/PathUtil.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/PathUtil.java index f4a91c237b..9f84a63851 100644 --- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/PathUtil.java +++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/PathUtil.java @@ -18,6 +18,10 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; * Basic {@link InstanceIdentifier} related tools. */ public class PathUtil { + + private PathUtil() { + throw new IllegalStateException("This class should not be instantiated."); + } public static NodeId digNodeId(final InstanceIdentifier nodeIdent) { return nodeIdent.firstKeyOf(Node.class, NodeKey.class).getId(); } diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/ReconcileUtil.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/ReconcileUtil.java index 79fd17a697..a910c01957 100644 --- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/ReconcileUtil.java +++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/ReconcileUtil.java @@ -23,7 +23,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; -import javax.annotation.Nullable; import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.action.GroupActionCase; import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.list.Action; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowCapableNode; @@ -50,41 +49,39 @@ import org.slf4j.LoggerFactory; /** * Util methods for group reconcil task (future chaining, transforms). */ -public class ReconcileUtil { +public final class ReconcileUtil { private static final Logger LOG = LoggerFactory.getLogger(ReconcileUtil.class); + private ReconcileUtil() { + throw new IllegalStateException("This class should not be instantiated."); + } + /** * @param previousItemAction description for case when the triggering future contains failure * @param type of rpc output (gathered in list) * @return single rpc result of type Void honoring all partial rpc results */ public static Function>, RpcResult> createRpcResultCondenser(final String previousItemAction) { - return new Function>, RpcResult>() { - @Nullable - @Override - public RpcResult apply(@Nullable final List> input) { - final RpcResultBuilder resultSink; - if (input != null) { - List errors = new ArrayList<>(); - for (RpcResult rpcResult : input) { - if (!rpcResult.isSuccessful()) { - errors.addAll(rpcResult.getErrors()); - } - } - if (errors.isEmpty()) { - resultSink = RpcResultBuilder.success(); - } else { - resultSink = RpcResultBuilder.failed().withRpcErrors(errors); + return input -> { + final RpcResultBuilder resultSink; + if (input != null) { + List errors = new ArrayList<>(); + for (RpcResult rpcResult : input) { + if (!rpcResult.isSuccessful()) { + errors.addAll(rpcResult.getErrors()); } + } + if (errors.isEmpty()) { + resultSink = RpcResultBuilder.success(); } else { - resultSink = RpcResultBuilder.failed() - .withError(RpcError.ErrorType.APPLICATION, "previous " + previousItemAction + " failed"); - + resultSink = RpcResultBuilder.failed().withRpcErrors(errors); } - - return resultSink.build(); + } else { + resultSink = RpcResultBuilder.failed() + .withError(RpcError.ErrorType.APPLICATION, "previous " + previousItemAction + " failed"); } + return resultSink.build(); }; } @@ -94,27 +91,21 @@ public class ReconcileUtil { * @return single rpc result of type Void honoring all partial rpc results */ public static Function, RpcResult> createRpcResultToVoidFunction(final String actionDescription) { - return new Function, RpcResult>() { - @Nullable - @Override - public RpcResult apply(@Nullable final RpcResult input) { - final RpcResultBuilder resultSink; - if (input != null) { - List errors = new ArrayList<>(); - if (!input.isSuccessful()) { - errors.addAll(input.getErrors()); - resultSink = RpcResultBuilder.failed().withRpcErrors(errors); - } else { - resultSink = RpcResultBuilder.success(); - } + return input -> { + final RpcResultBuilder resultSink; + if (input != null) { + List errors = new ArrayList<>(); + if (!input.isSuccessful()) { + errors.addAll(input.getErrors()); + resultSink = RpcResultBuilder.failed().withRpcErrors(errors); } else { - resultSink = RpcResultBuilder.failed() - .withError(RpcError.ErrorType.APPLICATION, "action of " + actionDescription + " failed"); - + resultSink = RpcResultBuilder.success(); } - - return resultSink.build(); + } else { + resultSink = RpcResultBuilder.failed() + .withError(RpcError.ErrorType.APPLICATION, "action of " + actionDescription + " failed"); } + return resultSink.build(); }; } @@ -157,7 +148,6 @@ public class ReconcileUtil { final Map installedGroupsArg, final Collection pendingGroups, final boolean gatherUpdates) { - final Map installedGroups = new HashMap<>(installedGroupsArg); final List> plan = new ArrayList<>(); diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/SwitchFlowId.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/SwitchFlowId.java index 5a9fbbaa2d..8ccc9b203f 100644 --- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/SwitchFlowId.java +++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/SwitchFlowId.java @@ -29,6 +29,26 @@ public class SwitchFlowId { this.match = flow.getMatch(); } + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + SwitchFlowId that = (SwitchFlowId) o; + + if (tableId != null ? !tableId.equals(that.tableId) : that.tableId != null) { + return false; + } + if (priority != null ? !priority.equals(that.priority) : that.priority != null) { + return false; + } + return match != null ? match.equals(that.match) : that.match == null; + } + @Override public int hashCode() { final int prime = 31; @@ -39,30 +59,4 @@ public class SwitchFlowId { return result; } - @Override - public boolean equals(Object obj) { - if (this == obj) - return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - SwitchFlowId other = (SwitchFlowId) obj; - if (match == null) { - if (other.match != null) - return false; - } else if (!match.equals(other.match)) - return false; - if (priority == null) { - if (other.priority != null) - return false; - } else if (!priority.equals(other.priority)) - return false; - if (tableId == null) { - if (other.tableId != null) - return false; - } else if (!tableId.equals(other.tableId)) - return false; - return true; - } } diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/SyncupEntry.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/SyncupEntry.java index 151724c563..290730e4cc 100644 --- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/SyncupEntry.java +++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/util/SyncupEntry.java @@ -71,4 +71,14 @@ public class SyncupEntry { return dsTypeBefore == that.dsTypeBefore; } + @Override + public int hashCode() { + final int prime = 31; + int result = after != null ? after.hashCode() : 0; + result = prime * result + (dsTypeAfter != null ? dsTypeAfter.hashCode() : 0); + result = prime * result + (before != null ? before.hashCode() : 0); + result = prime * result + (dsTypeBefore != null ? dsTypeBefore.hashCode() : 0); + return result; + } + } diff --git a/applications/forwardingrules-sync/src/test/java/org/opendaylight/openflowplugin/applications/frsync/impl/ForwardingRulesSyncProviderTest.java b/applications/forwardingrules-sync/src/test/java/org/opendaylight/openflowplugin/applications/frsync/impl/ForwardingRulesSyncProviderTest.java index b795b2f728..a9a649462a 100644 --- a/applications/forwardingrules-sync/src/test/java/org/opendaylight/openflowplugin/applications/frsync/impl/ForwardingRulesSyncProviderTest.java +++ b/applications/forwardingrules-sync/src/test/java/org/opendaylight/openflowplugin/applications/frsync/impl/ForwardingRulesSyncProviderTest.java @@ -7,6 +7,7 @@ */ package org.opendaylight.openflowplugin.applications.frsync.impl; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -71,4 +72,10 @@ public class ForwardingRulesSyncProviderTest { Matchers.>any(), Matchers.>any()); } + + @After + public void tearDown() throws InterruptedException { + provider.close(); + } + } \ No newline at end of file -- 2.36.6