Cleanup liblldp

Remove use of Class.newInstance() in favor of Supplier<Packet>,
which takes reflection out of the picture.

Also cleanup various other warnings, improving efficiency in the
process.

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

diff --git a/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/BitBufferHelper.java b/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/BitBufferHelper.java
index d3bcb1c3fdf6303b230a409298d2908cd30b7587..90cfdaaf0fb19fb925ee0625ce088165784b58fa 100644 (file)
--- a/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/BitBufferHelper.java
+++ b/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/BitBufferHelper.java
@@ -163,8 +163,8 @@ public abstract class BitBufferHelper {
      *             when the startOffset and numBits parameters are not congruent
      *             with the data buffer size
      */
-    @NonNull
-    public static byte[] getBits(final byte[] data, final int startOffset, final int numBits) throws BufferException {
+    public static byte @NonNull [] getBits(final byte[] data, final int startOffset, final int numBits)
+            throws BufferException {
         int startByteOffset;
         int extranumBits = numBits % NetUtils.NUM_BITS_IN_A_BYTE;
         final int extraOffsetBits = startOffset % NetUtils.NUM_BITS_IN_A_BYTE;
@@ -297,7 +297,8 @@ public abstract class BitBufferHelper {
         copyBits(src, dest, count, 0, startOffset);
     }
 
-    private static void copyBits(byte[] src, byte[] dest, int count, int srcBitIndex, int destBitIndex) {
+    private static void copyBits(final byte[] src, final byte[] dest, final int count,
+            int srcBitIndex, int destBitIndex) {
         int bitsRemaining = count;
         while (bitsRemaining > 0) {
             // How many bits can we, and do we need to, write, this time round?
@@ -307,13 +308,13 @@ public abstract class BitBufferHelper {
             }
             int targetByteIndex = destBitIndex / Byte.SIZE;
             int targetBitIndexInByte = destBitIndex % Byte.SIZE;
-            if (targetBitIndexInByte > 0 && (Byte.SIZE - targetBitIndexInByte) < bitsToCopy) {
+            if (targetBitIndexInByte > 0 && Byte.SIZE - targetBitIndexInByte < bitsToCopy) {
                 // We can't write that many bits
                 bitsToCopy = Byte.SIZE - targetBitIndexInByte;
             }
             int sourceByteIndex = srcBitIndex / Byte.SIZE;
             int sourceBitIndexInByte = srcBitIndex % Byte.SIZE;
-            if (sourceBitIndexInByte > 0 && (Byte.SIZE - sourceBitIndexInByte) < bitsToCopy) {
+            if (sourceBitIndexInByte > 0 && Byte.SIZE - sourceBitIndexInByte < bitsToCopy) {
                 // We can't read that many bits
                 bitsToCopy = Byte.SIZE - sourceBitIndexInByte;
             }

diff --git a/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/Ethernet.java b/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/Ethernet.java
index 3b5302f7756ff04c6a56fcb4a52f0c3a55c649ad..9e224a8d43dfbf8e66d73d4c771ebf5e6d91c067 100644 (file)
--- a/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/Ethernet.java
+++ b/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/Ethernet.java
@@ -5,12 +5,12 @@
  * 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.libraries.liblldp;
 
+import com.google.common.collect.ImmutableMap;
 import java.util.HashMap;
-import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.function.Supplier;
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.apache.commons.lang3.tuple.Pair;
 
@@ -24,27 +24,20 @@ public class Ethernet extends Packet {
 
     // TODO: This has to be outside and it should be possible for osgi
     // to add new coming packet classes
-    private static final Map<Short, Class<? extends Packet>> ETHER_TYPE_CLASS_MAP = new HashMap<>();
-
-    static {
-        ETHER_TYPE_CLASS_MAP.put(EtherTypes.LLDP.shortValue(), LLDP.class);
-    }
+    private static final Map<Short, Supplier<Packet>> ETHER_TYPE_CLASS_MAP = ImmutableMap.of(
+        EtherTypes.LLDP.shortValue(), LLDP::new);
 
-    private static final Map<String, Pair<Integer, Integer>> FIELD_COORDINATES = new LinkedHashMap<>();
-
-    static {
-        FIELD_COORDINATES.put(DMAC, new ImmutablePair<>(0, 48));
-        FIELD_COORDINATES.put(SMAC, new ImmutablePair<>(48, 48));
-        FIELD_COORDINATES.put(ETHT, new ImmutablePair<>(96, 16));
-    }
+    private static final Map<String, Pair<Integer, Integer>> FIELD_COORDINATES = ImmutableMap.of(
+        DMAC, new ImmutablePair<>(0, 48),
+        SMAC, new ImmutablePair<>(48, 48),
+        ETHT, new ImmutablePair<>(96, 16));
 
-    private final Map<String, byte[]> fieldValues;
+    private final Map<String, byte[]> fieldValues = new HashMap<>(4);
 
     /**
      * Default constructor that creates and sets the HashMap.
      */
     public Ethernet() {
-        fieldValues = new HashMap<>();
         hdrFieldCoordMap = FIELD_COORDINATES;
         hdrFieldsMap = fieldValues;
     }
@@ -54,7 +47,6 @@ public class Ethernet extends Packet {
      */
     public Ethernet(final boolean writeAccess) {
         super(writeAccess);
-        fieldValues = new HashMap<>();
         hdrFieldCoordMap = FIELD_COORDINATES;
         hdrFieldsMap = fieldValues;
     }
@@ -62,8 +54,7 @@ public class Ethernet extends Packet {
     @Override
     public void setHeaderField(final String headerField, final byte[] readValue) {
         if (headerField.equals(ETHT)) {
-            payloadClass = ETHER_TYPE_CLASS_MAP.get(BitBufferHelper
-                    .getShort(readValue));
+            payloadFactory = ETHER_TYPE_CLASS_MAP.get(BitBufferHelper.getShort(readValue));
         }
         hdrFieldsMap.put(headerField, readValue);
     }

diff --git a/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/LLDP.java b/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/LLDP.java
index 696562c1affb08acef2525f1c1af6a6f261e4eec..ec643f7f48f622db318c75f5750228bec08f0f6a 100644 (file)
--- a/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/LLDP.java
+++ b/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/LLDP.java
@@ -5,7 +5,6 @@
  * 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.libraries.liblldp;
 
 import com.google.common.collect.Iterables;
@@ -34,15 +33,15 @@ public class LLDP extends Packet {
     @SuppressFBWarnings("MS_PKGPROTECT")
     public static final byte[] LLDP_MULTICAST_MAC = { 1, (byte) 0x80, (byte) 0xc2, 0, 0, (byte) 0xe };
 
-    private Map<Byte, LLDPTLV> mandatoryTLVs;
-    private Map<Byte, LLDPTLV> optionalTLVs;
-    private Map<CustomTLVKey, LLDPTLV> customTLVs;
+    private final Map<Byte, LLDPTLV> mandatoryTLVs = new LinkedHashMap<>(LLDP_DEFAULT_TLVS);
+    private final Map<Byte, LLDPTLV> optionalTLVs = new LinkedHashMap<>();
+    private final Map<CustomTLVKey, LLDPTLV> customTLVs = new LinkedHashMap<>();
 
     /**
      * Default constructor that creates the tlvList LinkedHashMap.
      */
     public LLDP() {
-        init();
+
     }
 
     /**
@@ -50,13 +49,6 @@ public class LLDP extends Packet {
      */
     public LLDP(final boolean writeAccess) {
         super(writeAccess);
-        init();
-    }
-
-    private void init() {
-        mandatoryTLVs = new LinkedHashMap<>(LLDP_DEFAULT_TLVS);
-        optionalTLVs = new LinkedHashMap<>();
-        customTLVs = new LinkedHashMap<>();
     }
 
     /**
@@ -65,7 +57,7 @@ public class LLDP extends Packet {
      * @param typeDesc description of the type of TLV
      * @return byte type of TLV
      */
-    private byte getType(final String typeDesc) {
+    private static byte getType(final String typeDesc) {
         switch (typeDesc) {
             case CHASSISID:
                 return LLDPTLV.TLVType.ChassisID.getValue();

diff --git a/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/LLDPTLV.java b/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/LLDPTLV.java
index 6a715a9b42f667cb704f2a452079723ff3873012..d8516f60d78e6029bab1707063a2bc417341a711 100644 (file)
--- a/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/LLDPTLV.java
+++ b/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/LLDPTLV.java
@@ -5,27 +5,24 @@
  * 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.libraries.liblldp;
 
+import com.google.common.collect.ImmutableMap;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.HashMap;
-import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.Objects;
 import org.apache.commons.lang3.ArrayUtils;
 import org.apache.commons.lang3.tuple.MutablePair;
 import org.apache.commons.lang3.tuple.Pair;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Class that represents the LLDPTLV objects.
  */
 @SuppressWarnings("checkstyle:AbbreviationAsWordInName")
 public class LLDPTLV extends Packet {
-    private static final Logger LOG = LoggerFactory.getLogger(LLDPTLV.class);
     private static final String TYPE = "Type";
     private static final String LENGTH = "Length";
     private static final String VALUE = "Value";
@@ -63,13 +60,10 @@ public class LLDPTLV extends Packet {
         }
     }
 
-    private static final Map<String, Pair<Integer, Integer>> FIELD_COORDINATES = new LinkedHashMap<>();
-
-    static {
-        FIELD_COORDINATES.put(TYPE, new MutablePair<>(0, 7));
-        FIELD_COORDINATES.put(LENGTH, new MutablePair<>(7, 9));
-        FIELD_COORDINATES.put(VALUE, new MutablePair<>(16, 0));
-    }
+    private static final Map<String, Pair<Integer, Integer>> FIELD_COORDINATES = ImmutableMap.of(
+        TYPE, new MutablePair<>(0, 7),
+        LENGTH, new MutablePair<>(7, 9),
+        VALUE, new MutablePair<>(16, 0));
 
     protected Map<String, byte[]> fieldValues;
 
@@ -174,11 +168,7 @@ public class LLDPTLV extends Packet {
             return false;
         }
         LLDPTLV other = (LLDPTLV) obj;
-        if (fieldValues == null) {
-            if (other.fieldValues != null) {
-                return false;
-            }
-        } else if (!fieldValues.equals(other.fieldValues)) {
+        if (!Objects.equals(fieldValues, other.fieldValues)) {
             return false;
         }
         return true;

diff --git a/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/Packet.java b/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/Packet.java
index bcff15bf2d5f1425634c92824d0ff58a75e5d8e3..2822b2736f1d765ec0310a60d58df4f98022099a 100644 (file)
--- a/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/Packet.java
+++ b/libraries/liblldp/src/main/java/org/opendaylight/openflowplugin/libraries/liblldp/Packet.java
@@ -11,6 +11,7 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Arrays;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.function.Supplier;
 import org.apache.commons.lang3.tuple.Pair;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -46,7 +47,7 @@ public abstract class Packet {
     protected Map<String, byte[]> hdrFieldsMap;
 
     // The class of the encapsulated packet object
-    protected Class<? extends Packet> payloadClass;
+    protected Supplier<Packet> payloadFactory;
 
     public Packet() {
         writeAccess = false;
@@ -125,12 +126,8 @@ public abstract class Packet {
         int payloadStart = startOffset + numBits;
         int payloadSize = data.length * NetUtils.NUM_BITS_IN_A_BYTE - payloadStart;
 
-        if (payloadClass != null) {
-            try {
-                payload = payloadClass.newInstance();
-            } catch (InstantiationException | IllegalAccessException e) {
-                throw new PacketException("Error parsing payload for Ethernet packet", e);
-            }
+        if (payloadFactory != null) {
+            payload = payloadFactory.get();
             payload.deserialize(data, payloadStart, payloadSize);
             payload.setParent(this);
         } else {
@@ -143,7 +140,6 @@ public abstract class Packet {
             rawPayload = Arrays.copyOfRange(data, start, stop);
         }
 
-
         // Take care of computation that can be done only after deserialization
         postDeserializeCustomOperation(data, payloadStart - getHeaderSize());
 
@@ -212,7 +208,7 @@ public abstract class Packet {
      * @param myBytes serialized bytes
      * @throws PacketException on failure
      */
-    protected void postSerializeCustomOperation(byte[] myBytes) throws PacketException {
+    protected void postSerializeCustomOperation(final byte[] myBytes) throws PacketException {
         // no op
     }
 
@@ -227,7 +223,7 @@ public abstract class Packet {
      * @param startBitOffset The bit offset from where the byte array corresponding to this Packet starts in the frame
      * @throws PacketException on failure
      */
-    protected void postDeserializeCustomOperation(byte[] data, int startBitOffset) throws PacketException {
+    protected void postDeserializeCustomOperation(final byte[] data, final int startBitOffset) throws PacketException {
         // no op
     }