Merge "OPNFLWPLUG-898 Improve code quality in liblldp module"

diff --git a/applications/topology-lldp-discovery/src/main/java/org/opendaylight/openflowplugin/applications/topology/lldp/utils/LLDPDiscoveryUtils.java b/applications/topology-lldp-discovery/src/main/java/org/opendaylight/openflowplugin/applications/topology/lldp/utils/LLDPDiscoveryUtils.java
index f7d2e475dcf910865e1b3189993e4ad9ef6bc335..39288e64ba51ac8dc059c0e1754a9609bccbd9ce 100644 (file)
--- a/applications/topology-lldp-discovery/src/main/java/org/opendaylight/openflowplugin/applications/topology/lldp/utils/LLDPDiscoveryUtils.java
+++ b/applications/topology-lldp-discovery/src/main/java/org/opendaylight/openflowplugin/applications/topology/lldp/utils/LLDPDiscoveryUtils.java
@@ -130,7 +130,7 @@ public final class LLDPDiscoveryUtils {
                 InstanceIdentifier<NodeConnector> srcInstanceId = InstanceIdentifier.builder(Nodes.class)
                         .child(Node.class, new NodeKey(srcNodeId))
                         .child(NodeConnector.class, new NodeConnectorKey(srcNodeConnectorId))
-                        .toInstance();
+                        .build();
                 nodeConnectorRef = new NodeConnectorRef(srcInstanceId);
             } catch (Exception e) {
                 LOG.debug("Caught exception while parsing out lldp optional and custom fields", e);

diff --git a/extension/openflowjava-extension-nicira/src/main/java/org/opendaylight/openflowjava/nx/codec/action/LearnCodecUtil.java b/extension/openflowjava-extension-nicira/src/main/java/org/opendaylight/openflowjava/nx/codec/action/LearnCodecUtil.java
index 3da7e8a86bfc5ba6bcfd473464ad73be8161cfff..a453440a2001ea41ea2d19b934ef5d886340ef18 100644 (file)
--- a/extension/openflowjava-extension-nicira/src/main/java/org/opendaylight/openflowjava/nx/codec/action/LearnCodecUtil.java
+++ b/extension/openflowjava-extension-nicira/src/main/java/org/opendaylight/openflowjava/nx/codec/action/LearnCodecUtil.java
@@ -252,9 +252,9 @@ public final class LearnCodecUtil {
 
     private static FlowMods readFlowModAddMatchFromField(ByteBuf message, short numBits) {
         FlowModAddMatchFromFieldBuilder builder = new FlowModAddMatchFromFieldBuilder();
-        builder.setSrcField((long) message.readInt());
+        builder.setSrcField(message.readUnsignedInt());
         builder.setSrcOfs((int) message.readShort());
-        builder.setDstField((long) message.readInt());
+        builder.setDstField(message.readUnsignedInt());
         builder.setDstOfs((int) message.readShort());
         builder.setFlowModNumBits((int) numBits);
         length -= FROM_FIELD_LENGTH - EncodeConstants.SIZE_OF_SHORT_IN_BYTES;
@@ -269,7 +269,7 @@ public final class LearnCodecUtil {
     private static FlowMods readFlowModAddMatchFromValue(ByteBuf message, short numBits) {
         FlowModAddMatchFromValueBuilder builder = new FlowModAddMatchFromValueBuilder();
         builder.setValue(message.readUnsignedShort());
-        builder.setSrcField((long) message.readInt());
+        builder.setSrcField(message.readUnsignedInt());
         builder.setSrcOfs((int) message.readShort());
         builder.setFlowModNumBits((int) numBits);
         length -= FROM_VALUE_LENGTH - EncodeConstants.SIZE_OF_SHORT_IN_BYTES;
@@ -283,9 +283,9 @@ public final class LearnCodecUtil {
 
     private static FlowMods readFlowModCopyFromField(ByteBuf message, short numBits) {
         FlowModCopyFieldIntoFieldBuilder builder = new FlowModCopyFieldIntoFieldBuilder();
-        builder.setSrcField((long) message.readInt());
+        builder.setSrcField(message.readUnsignedInt());
         builder.setSrcOfs((int) message.readShort());
-        builder.setDstField((long) message.readInt());
+        builder.setDstField(message.readUnsignedInt());
         builder.setDstOfs((int) message.readShort());
         builder.setFlowModNumBits((int) numBits);
         length -= FROM_FIELD_LENGTH - EncodeConstants.SIZE_OF_SHORT_IN_BYTES;
@@ -300,7 +300,7 @@ public final class LearnCodecUtil {
     private static FlowMods readFlowModCopyFromValue(ByteBuf message, short numBits) {
         FlowModCopyValueIntoFieldBuilder builder = new FlowModCopyValueIntoFieldBuilder();
         builder.setValue(message.readUnsignedShort());
-        builder.setDstField((long) message.readInt());
+        builder.setDstField(message.readUnsignedInt());
         builder.setDstOfs((int) message.readShort());
         builder.setFlowModNumBits((int) numBits);
         length -= FROM_VALUE_LENGTH - EncodeConstants.SIZE_OF_SHORT_IN_BYTES;
@@ -314,7 +314,7 @@ public final class LearnCodecUtil {
 
     private static FlowMods readFlowToPort(ByteBuf message, short numBits) {
         FlowModOutputToPortBuilder builder = new FlowModOutputToPortBuilder();
-        builder.setSrcField((long) message.readInt());
+        builder.setSrcField(message.readUnsignedInt());
         builder.setSrcOfs((int) message.readShort());
         builder.setFlowModNumBits((int) numBits);
         length -= TO_PORT_LENGTH - EncodeConstants.SIZE_OF_SHORT_IN_BYTES;

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/HandshakeManagerImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/HandshakeManagerImpl.java
index e2f9d91f973129315acd441c4d77579eab1e5a6a..e2107dfb73265d0df8be274324330ca1f64e991e 100644 (file)
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/HandshakeManagerImpl.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/HandshakeManagerImpl.java
@@ -169,7 +169,7 @@ public class HandshakeManagerImpl implements HandshakeManager {
     }
 
     private void stepByStepVersionSubStep(Short remoteVersion) throws Exception {
-        if (remoteVersion.equals(lastProposedVersion)) {
+        if (remoteVersion >= lastProposedVersion) {
             postHandshake(lastProposedVersion, getNextXid());
             LOG.trace("ret - OK - switch answered with lastProposedVersion");
         } else {

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java
index 67e113edf7fe11adca681668d39fe4d9a0b987a4..f5f812157ee3f1dc0078a0c1ecec4946d410746a 100644 (file)
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/registry/flow/DeviceFlowRegistryImpl.java
@@ -290,9 +290,11 @@ public class DeviceFlowRegistryImpl implements DeviceFlowRegistry {
                 return flowRegistryKey;
             }
         } else {
-            for (Map.Entry<FlowRegistryKey, FlowDescriptor> keyValueSet : flowRegistry.entrySet()) {
-                if (keyValueSet.getKey().equals(flowRegistryKey)) {
-                    return keyValueSet.getKey();
+            synchronized (flowRegistry) {
+                for (Map.Entry<FlowRegistryKey, FlowDescriptor> keyValueSet : flowRegistry.entrySet()) {
+                    if (keyValueSet.getKey().equals(flowRegistryKey)) {
+                        return keyValueSet.getKey();
+                    }
                 }
             }
         }

diff --git a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/GroupConvertorTest.java b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/GroupConvertorTest.java
index 1209c264c95ee371ce781ce7380af39ebe232d2f..0a5ac4408cfa894f2996a788b09e66453e6f9bea 100644 (file)
--- a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/GroupConvertorTest.java
+++ b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/convertor/GroupConvertorTest.java
@@ -20,7 +20,6 @@ import org.junit.Test;
 import org.opendaylight.openflowplugin.openflow.md.core.sal.convertor.data.VersionDatapathIdConvertorData;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Uri;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.action.CopyTtlInCaseBuilder;
-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.action.GroupActionCaseBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.action.OutputActionCaseBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.action.SetMplsTtlActionCaseBuilder;
@@ -42,6 +41,8 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.group
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.group.BucketsBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.group.buckets.Bucket;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.group.buckets.BucketBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.action.grouping.action.choice.GroupCaseBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.action.grouping.action.choice.SetMplsTtlCaseBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.actions.grouping.Action;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev130731.GroupModCommand;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.types.rev130731.GroupType;
@@ -155,32 +156,30 @@ public class GroupConvertorTest {
         assertEquals((Long) 22L, outAddGroupInput.getBucketsList().get(0).getWatchGroup());
 
         final List<Action> outActionList = outAddGroupInput.getBucketsList().get(0).getAction();
-        for (int outItem = 0; outItem < outActionList.size(); outItem++) {
-            final Action action = outActionList
-                    .get(outItem);
-            if (action.getActionChoice() instanceof GroupActionCase) {
-                assertEquals((Long) 5L, ((GroupActionCase) action.getActionChoice()).getGroupAction().getGroupId());
-
-            }
-            // TODO:setMplsTTL :OF layer doesnt have get();
-        }
+        assertEquals(ImmutableList.of(new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action
+                    .rev150203.actions.grouping.ActionBuilder().setActionChoice(new GroupCaseBuilder().setGroupAction(
+                new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.action.grouping
+                        .action.choice.group._case.GroupActionBuilder().setGroupId(5L).build()).build()).build(),
+                    new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.actions.grouping
+                    .ActionBuilder().setActionChoice(new GroupCaseBuilder().setGroupAction(
+                            new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.action
+                            .grouping.action.choice.group._case.GroupActionBuilder().setGroupId(5L).build()).build())
+                    .build()), outActionList);
 
         assertEquals((Integer) 50, outAddGroupInput.getBucketsList().get(1).getWeight());
         assertEquals((long) 60, (long) outAddGroupInput.getBucketsList().get(1).getWatchPort().getValue());
         assertEquals((Long) 70L, outAddGroupInput.getBucketsList().get(1).getWatchGroup());
-        final List<Action> outActionList1 = outAddGroupInput.getBucketsList().get(1).getAction();
-        for (int outItem = 0; outItem < outActionList1.size(); outItem++) {
-            final Action action = outActionList1
-                    .get(outItem);
-            if (action.getActionChoice() instanceof GroupActionCase) {
-
-                assertEquals((Long) 6L, ((GroupActionCase) action.getActionChoice()).getGroupAction().getGroupId());
-
-
-            }
-            // TODO:setMplsTTL :OF layer doesnt have get();
-        }
 
+        final List<Action> outActionList1 = outAddGroupInput.getBucketsList().get(1).getAction();
+        assertEquals(ImmutableList.of(new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action
+                    .rev150203.actions.grouping.ActionBuilder().setActionChoice(new GroupCaseBuilder().setGroupAction(
+                new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.action.grouping
+                        .action.choice.group._case.GroupActionBuilder().setGroupId(5L).build()).build()).build(),
+                    new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.actions.grouping
+                    .ActionBuilder().setActionChoice(new GroupCaseBuilder().setGroupAction(
+                            new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.action
+                            .grouping.action.choice.group._case.GroupActionBuilder().setGroupId(5L).build()).build())
+                    .build()), outActionList1);
     }
 
     /**
@@ -266,22 +265,27 @@ public class GroupConvertorTest {
         assertEquals(10L, outAddGroupInput.getGroupId().getValue().longValue());
 
         final List<Action> outActionList = outAddGroupInput.getBucketsList().get(0).getAction();
-        for (int outItem = 0; outItem < outActionList.size(); outItem++) {
-            final Action action = outActionList
-                    .get(outItem);
-            if (action.getActionChoice() instanceof GroupActionCase) {
-                assertEquals((Long) 5L, ((GroupActionCase) action.getActionChoice()).getGroupAction().getGroupId());
-            }
-        }
+        assertEquals(ImmutableList.of(new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203
+                    .actions.grouping.ActionBuilder().setActionChoice(new GroupCaseBuilder().setGroupAction(
+                            new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.action
+                            .grouping.action.choice.group._case.GroupActionBuilder().setGroupId(5L).build()).build())
+                    .build(),
+                    new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.actions.grouping
+                            .ActionBuilder().setActionChoice(new GroupCaseBuilder().setGroupAction(
+                                    new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203
+                                    .action.grouping.action.choice.group._case.GroupActionBuilder().setGroupId(6L)
+                                    .build()).build()).build()), outActionList);
 
         final List<Action> outActionList1 = outAddGroupInput.getBucketsList().get(1).getAction();
-        for (int outItem = 0; outItem < outActionList1.size(); outItem++) {
-            final Action action = outActionList1
-                    .get(outItem);
-            if (action.getActionChoice() instanceof GroupActionCase) {
-                assertEquals((Long) 6L, ((GroupActionCase) action.getActionChoice()).getGroupAction().getGroupId());
-            }
-        }
+        assertEquals(ImmutableList.of(new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203
+                    .actions.grouping.ActionBuilder().setActionChoice(
+                            new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.action
+                            .grouping.action.choice.CopyTtlInCaseBuilder().build()).build(),
+                    new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.actions.grouping
+                            .ActionBuilder().setActionChoice(new SetMplsTtlCaseBuilder().setSetMplsTtlAction(
+                                    new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203
+                                            .action.grouping.action.choice.set.mpls.ttl._case.SetMplsTtlActionBuilder()
+                                            .setMplsTtl((short) 1).build()).build()).build()), outActionList1);
     }
 
     /**
@@ -488,31 +492,31 @@ public class GroupConvertorTest {
         assertEquals(10L, outAddGroupInput.getGroupId().getValue().longValue());
 
         final List<Action> outActionList = outAddGroupInput.getBucketsList().get(0).getAction();
-        for (int outItem = 0; outItem < outActionList.size(); outItem++) {
-            final Action action = outActionList
-                    .get(outItem);
-            if (action.getActionChoice() instanceof GroupActionCase) {
-                assertEquals((Long) 5L, ((GroupActionCase) action.getActionChoice()).getGroupAction().getGroupId());
-
-            }
-
-        }
+        assertEquals(ImmutableList.of(new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203
+                    .actions.grouping.ActionBuilder().setActionChoice(new GroupCaseBuilder().setGroupAction(
+                            new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.action
+                                    .grouping.action.choice.group._case.GroupActionBuilder().setGroupId(5L).build())
+                        .build()).build(),
+                    new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.actions.grouping
+                            .ActionBuilder().setActionChoice(new GroupCaseBuilder().setGroupAction(
+                                    new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203
+                                            .action.grouping.action.choice.group._case.GroupActionBuilder()
+                                            .setGroupId(6L).build()).build()).build()), outActionList);
 
         final List<Action> outActionList1 = outAddGroupInput.getBucketsList().get(1).getAction();
-        for (int outItem = 0; outItem < outActionList1.size(); outItem++) {
-            final Action action = outActionList1
-                    .get(outItem);
-            if (action.getActionChoice() instanceof GroupActionCase) {
-
-                assertEquals((Long) 6L, ((GroupActionCase) action.getActionChoice()).getGroupAction().getGroupId());
-
-            }
-
-        }
-
+        assertEquals(ImmutableList.of(new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203
+                    .actions.grouping.ActionBuilder().setActionChoice(new GroupCaseBuilder().setGroupAction(
+                            new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.action
+                                    .grouping.action.choice.group._case.GroupActionBuilder().setGroupId(5L).build())
+                        .build()).build(),
+                    new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203.actions.grouping
+                            .ActionBuilder().setActionChoice(new GroupCaseBuilder().setGroupAction(
+                                    new org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.common.action.rev150203
+                                            .action.grouping.action.choice.group._case.GroupActionBuilder()
+                                            .setGroupId(6L).build()).build()).build()), outActionList1);
     }
 
-    private GroupModInputBuilder convert(Group group, VersionDatapathIdConvertorData data) {
+    private GroupModInputBuilder convert(final Group group, final VersionDatapathIdConvertorData data) {
         final Optional<GroupModInputBuilder> outAddGroupInputOptional = convertorManager.convert(group, data);
         assertTrue("Group convertor not found", outAddGroupInputOptional.isPresent());
         return outAddGroupInputOptional.get();