PortMapping Refactoring step 2

This refactoring intends to optimize some parts of the code
and to fix sonar issues about cyclomatic complexity.
It introduces intermediate functions, sometimes by reevaluating log
levels and messages content.

JIRA: TRNSPRTPCE-355 TRNSPRTPCE-356
Signed-off-by: guillaume.lambert <guillaume.lambert@orange.com>
Change-Id: I625598fd935c296f59c9ad42966d5353c21a59d8

diff --git a/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingUtils.java b/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingUtils.java
index a525bb1aac2b36be9a8ee8a5d2d535d2f4140837..0a2ed8cc548dd1c5cf76e3f0fe4f5447d1b0982e 100644 (file)
--- a/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingUtils.java
+++ b/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingUtils.java
@@ -26,8 +26,6 @@ public final class PortMappingUtils {
         "{} : port {} on {} - associated Logical Connection Point is {}";
     public static final String CANNOT_AS_LCP_LOGMSG =
         " - cannot assign Logical Connection Point";
-    public static final String CANNOT_CREATE_LCP_LOGMSG =
-        "{} : port {} on {} - Impossible to create logical connection point";
     public static final String CANNOT_GET_DEV_CONF_LOGMSG =
         "{} : impossible to get device configuration";
     public static final String CANNOT_GET_LLDP_CONF_LOGMSG =
@@ -81,13 +79,11 @@ public final class PortMappingUtils {
     public static final String PORT_NOT_RDMEXT_LOGMSG =
         "{} : port {} on {} is not roadm-external";
     public static final String PORTDIR_ERROR_LOGMSG =
-        " - error in configuration with port-direction";
+        "{} : port {} on {} - error in configuration with port-direction";
     public static final String PORTMAPPING_IGNORE_LOGMSG =
         " - ignoring it in port-mapping";
     public static final String PORTMAPPING_POST_FAIL_LOGMSG =
         "{} : port-mapping post-treatment failure for {}";
-    public static final String PORTQUAL_ERROR_LOGMSG =
-        " - error in configuration with port-qual";
     public static final String PORTQUAL_LOGMSG =
         "{} : port {} on {} - PortQual {}";
     public static final String PROCESSING_DONE_LOGMSG =

diff --git a/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingVersion121.java b/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingVersion121.java
index 661a532bb95d9bbab9973a33c20f1510711adba6..409ad5ebfd73ab67b05c844092f91852aff2a6ce 100644 (file)
--- a/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingVersion121.java
+++ b/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingVersion121.java
@@ -18,6 +18,7 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.stream.Collectors;
 import org.eclipse.jdt.annotation.NonNull;
@@ -85,6 +86,7 @@ public class PortMappingVersion121 {
 
     private static final Logger LOG = LoggerFactory.getLogger(PortMappingVersion121.class);
     private static final Map<Direction, String> SUFFIX;
+    private static final Set<Integer> TXRX_SET = Set.of(Direction.Tx.getIntValue(), Direction.Rx.getIntValue());
 
     private final DataBroker dataBroker;
     private final DeviceTransactionManager deviceTransactionManager;
@@ -262,12 +264,9 @@ public class PortMappingVersion121 {
     }
 
     private boolean checkPartnerPort(String circuitPackName, Ports port1, Ports port2) {
-        return (checkPartnerPortNoDir(circuitPackName, port1, port2)
-            && ((Direction.Rx.getIntValue() == port1.getPortDirection().getIntValue()
-                    && Direction.Tx.getIntValue() == port2.getPortDirection().getIntValue())
-                ||
-                (Direction.Tx.getIntValue() == port1.getPortDirection().getIntValue()
-                    && Direction.Rx.getIntValue() == port2.getPortDirection().getIntValue())));
+        return checkPartnerPortNoDir(circuitPackName, port1, port2)
+            && Set.of(port1.getPortDirection().getIntValue(), port2.getPortDirection().getIntValue())
+                .equals(TXRX_SET);
     }
 
     private HashMap<Integer, List<org.opendaylight.yang.gen.v1.http.org.openroadm.device.rev170206.srg
@@ -329,17 +328,11 @@ public class PortMappingVersion121 {
                 Collections.sort(portList, new SortPort121ByName());
                 int portIndex = 1;
                 for (Ports port : portList) {
-                    String currentKey = circuitPackName + "-" + port.getPortName();
-                    if (port.getPortQual() == null) {
-                        continue;
-                    }
-
-                    if (Port.PortQual.RoadmExternal.getIntValue() != port.getPortQual().getIntValue()) {
-                        LOG.debug(PortMappingUtils.PORT_NOT_RDMEXT_LOGMSG + PortMappingUtils.CANNOT_AS_LCP_LOGMSG,
-                            nodeId, port.getPortName(), circuitPackName);
+                    if (!checkPortQual(port, circuitPackName, nodeId)) {
                         continue;
                     }
 
+                    String currentKey = circuitPackName + "-" + port.getPortName();
                     if (keys.contains(currentKey)) {
                         LOG.debug(PortMappingUtils.PORT_ALREADY_HANDLED_LOGMSG + PortMappingUtils.CANNOT_AS_LCP_LOGMSG,
                             nodeId, port.getPortName(), circuitPackName);
@@ -834,17 +827,7 @@ public class PortMappingVersion121 {
                         return false;
                     }
                     Ports port = portObject.get();
-                    if (port.getPortQual() == null) {
-                        continue;
-                    }
-                    if (Port.PortQual.RoadmExternal.getIntValue() != port.getPortQual().getIntValue()) {
-                        LOG.error(PortMappingUtils.CANNOT_CREATE_LCP_LOGMSG + PortMappingUtils.PORTQUAL_ERROR_LOGMSG,
-                            nodeId, port.getPortName(), cp1Name);
-                        continue;
-                    }
-                    if (Direction.Bidirectional.getIntValue() != port.getPortDirection().getIntValue()) {
-                        LOG.error(PortMappingUtils.CANNOT_CREATE_LCP_LOGMSG + PortMappingUtils.PORTDIR_ERROR_LOGMSG,
-                            nodeId, port.getPortName(), cp1Name);
+                    if (!checkTtpPort(port, cp1Name, nodeId, true)) {
                         continue;
                     }
 
@@ -868,8 +851,7 @@ public class PortMappingVersion121 {
                     InstanceIdentifier<Ports> port2ID = InstanceIdentifier.create(OrgOpenroadmDevice.class)
                         .child(CircuitPacks.class, new CircuitPacksKey(cp2Name))
                         .child(Ports.class, new PortsKey(cp2.getPortName()));
-                    LOG.debug(PortMappingUtils.FETCH_CONNECTIONPORT_LOGMSG,
-                        nodeId, cp2.getPortName(), cp2Name);
+                    LOG.debug(PortMappingUtils.FETCH_CONNECTIONPORT_LOGMSG, nodeId, cp2.getPortName(), cp2Name);
                     Optional<Ports> port2Object = this.deviceTransactionManager.getDataFromDevice(nodeId,
                         LogicalDatastoreType.OPERATIONAL, port2ID, Timeouts.DEVICE_READ_TIMEOUT,
                         Timeouts.DEVICE_READ_TIMEOUT_UNIT);
@@ -883,18 +865,11 @@ public class PortMappingVersion121 {
                     }
 
                     Ports port1 = port1Object.get();
-                    Ports port2 = port2Object.get();
-                    if (port1.getPortQual() == null || port2.getPortQual() == null) {
+                    if (!checkTtpPort(port1, cp1Name, nodeId, false)) {
                         continue;
                     }
-                    if (Port.PortQual.RoadmExternal.getIntValue() != port1.getPortQual().getIntValue()) {
-                        LOG.error(PortMappingUtils.CANNOT_CREATE_LCP_LOGMSG + PortMappingUtils.PORTQUAL_ERROR_LOGMSG,
-                            nodeId, port1.getPortName(), cp1Name);
-                        continue;
-                    }
-                    if (Port.PortQual.RoadmExternal.getIntValue() != port2.getPortQual().getIntValue()) {
-                        LOG.error(PortMappingUtils.CANNOT_CREATE_LCP_LOGMSG + PortMappingUtils.PORTQUAL_ERROR_LOGMSG,
-                            nodeId, port2.getPortName(), cp2Name);
+                    Ports port2 = port2Object.get();
+                    if (!checkTtpPort(port2, cp2Name, nodeId, false)) {
                         continue;
                     }
 
@@ -929,6 +904,32 @@ public class PortMappingVersion121 {
         return true;
     }
 
+    private boolean checkPortQual(Ports port, String cpName, String nodeId) {
+        if (port.getPortQual() == null) {
+            return false;
+        }
+        if (Port.PortQual.RoadmExternal.getIntValue() != port.getPortQual().getIntValue()) {
+            //used to be LOG.error when called from createTtpPortMapping
+            LOG.debug(PortMappingUtils.PORT_NOT_RDMEXT_LOGMSG + PortMappingUtils.CANNOT_AS_LCP_LOGMSG,
+                nodeId, port.getPortName(), cpName);
+            return false;
+        }
+        return true;
+    }
+
+    private boolean checkTtpPort(Ports port, String cpName, String nodeId, boolean bidirectional) {
+        if (!checkPortQual(port, cpName, nodeId)) {
+            return false;
+        }
+        if (Direction.Bidirectional.getIntValue() == port.getPortDirection().getIntValue() ^ bidirectional) {
+        // (a ^ b) makes more sense than (!a && b) here since it can also work for unidirectional links
+            LOG.error(PortMappingUtils.PORTDIR_ERROR_LOGMSG + PortMappingUtils.CANNOT_AS_LCP_LOGMSG,
+                nodeId, port.getPortName(), cpName);
+            return false;
+        }
+        return true;
+    }
+
     private NodeInfo createNodeInfo(Info deviceInfo) {
 
         if (deviceInfo.getNodeType() == null) {

diff --git a/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingVersion221.java b/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingVersion221.java
index cbc4eb575c2cac4064ac84d0280952aafc10c27c..d2dcfc883270eef9951dc85f0b6fbb6e5bfabf72 100644 (file)
--- a/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingVersion221.java
+++ b/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingVersion221.java
@@ -18,6 +18,7 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.stream.Collectors;
 import org.eclipse.jdt.annotation.NonNull;
@@ -101,6 +102,7 @@ public class PortMappingVersion221 {
 
     private static final Logger LOG = LoggerFactory.getLogger(PortMappingVersion221.class);
     private static final Map<Direction, String> SUFFIX;
+    private static final Set<Integer> TXRX_SET = Set.of(Direction.Tx.getIntValue(), Direction.Rx.getIntValue());
 
     private final DataBroker dataBroker;
     private final DeviceTransactionManager deviceTransactionManager;
@@ -374,12 +376,9 @@ public class PortMappingVersion221 {
     }
 
     private boolean checkPartnerPort(String circuitPackName, Ports port1, Ports port2) {
-        return (checkPartnerPortNoDir(circuitPackName, port1, port2)
-            && ((Direction.Rx.getIntValue() == port1.getPortDirection().getIntValue()
-                    && Direction.Tx.getIntValue() == port2.getPortDirection().getIntValue())
-                ||
-                (Direction.Tx.getIntValue() == port1.getPortDirection().getIntValue()
-                    && Direction.Rx.getIntValue() == port2.getPortDirection().getIntValue())));
+        return checkPartnerPortNoDir(circuitPackName, port1, port2)
+            && Set.of(port1.getPortDirection().getIntValue(), port2.getPortDirection().getIntValue())
+                .equals(TXRX_SET);
     }
 
 
@@ -439,17 +438,11 @@ public class PortMappingVersion221 {
                 Collections.sort(portList, new SortPort221ByName());
                 int portIndex = 1;
                 for (Ports port : portList) {
-                    String currentKey = circuitPackName + "-" + port.getPortName();
-                    if (port.getPortQual() == null) {
-                        continue;
-                    }
-
-                    if (PortQual.RoadmExternal.getIntValue() != port.getPortQual().getIntValue()) {
-                        LOG.debug(PortMappingUtils.PORT_NOT_RDMEXT_LOGMSG + PortMappingUtils.CANNOT_AS_LCP_LOGMSG,
-                            nodeId, port.getPortName(), circuitPackName);
+                    if (!checkPortQual(port, circuitPackName, nodeId)) {
                         continue;
                     }
 
+                    String currentKey = circuitPackName + "-" + port.getPortName();
                     if (keys.contains(currentKey)) {
                         LOG.debug(PortMappingUtils.PORT_ALREADY_HANDLED_LOGMSG + PortMappingUtils.CANNOT_AS_LCP_LOGMSG,
                             nodeId, port.getPortName(), circuitPackName);
@@ -1056,17 +1049,7 @@ public class PortMappingVersion221 {
                         return false;
                     }
                     Ports port = portObject.get();
-                    if (port.getPortQual() == null) {
-                        continue;
-                    }
-                    if (PortQual.RoadmExternal.getIntValue() != port.getPortQual().getIntValue()) {
-                        LOG.error(PortMappingUtils.CANNOT_CREATE_LCP_LOGMSG + PortMappingUtils.PORTQUAL_ERROR_LOGMSG,
-                            nodeId, port.getPortName(), cp1Name);
-                        continue;
-                    }
-                    if (Direction.Bidirectional.getIntValue() != port.getPortDirection().getIntValue()) {
-                        LOG.error(PortMappingUtils.CANNOT_CREATE_LCP_LOGMSG + PortMappingUtils.PORTDIR_ERROR_LOGMSG,
-                            nodeId, port.getPortName(), cp1Name);
+                    if (!checkTtpPort(port, cp1Name, nodeId, true)) {
                         continue;
                     }
 
@@ -1104,18 +1087,11 @@ public class PortMappingVersion221 {
                     }
 
                     Ports port1 = port1Object.get();
-                    Ports port2 = port2Object.get();
-                    if (port1.getPortQual() == null || port2.getPortQual() == null) {
+                    if (!checkTtpPort(port1, cp1Name, nodeId, false)) {
                         continue;
                     }
-                    if (PortQual.RoadmExternal.getIntValue() != port1.getPortQual().getIntValue()) {
-                        LOG.error(PortMappingUtils.CANNOT_CREATE_LCP_LOGMSG + PortMappingUtils.PORTQUAL_ERROR_LOGMSG,
-                            nodeId, port1.getPortName(), cp1Name);
-                        continue;
-                    }
-                    if (PortQual.RoadmExternal.getIntValue() != port2.getPortQual().getIntValue()) {
-                        LOG.error(PortMappingUtils.CANNOT_CREATE_LCP_LOGMSG + PortMappingUtils.PORTQUAL_ERROR_LOGMSG,
-                            nodeId, port2.getPortName(), cp2Name);
+                    Ports port2 = port2Object.get();
+                    if (!checkTtpPort(port2, cp2Name, nodeId, false)) {
                         continue;
                     }
 
@@ -1150,6 +1126,32 @@ public class PortMappingVersion221 {
         return true;
     }
 
+    private boolean checkPortQual(Ports port, String cpName, String nodeId) {
+        if (port.getPortQual() == null) {
+            return false;
+        }
+        if (PortQual.RoadmExternal.getIntValue() != port.getPortQual().getIntValue()) {
+            //used to be LOG.error when called from createTtpPortMapping
+            LOG.debug(PortMappingUtils.PORT_NOT_RDMEXT_LOGMSG + PortMappingUtils.CANNOT_AS_LCP_LOGMSG,
+                nodeId, port.getPortName(), cpName);
+            return false;
+        }
+        return true;
+    }
+
+    private boolean checkTtpPort(Ports port, String cpName, String nodeId, boolean bidirectional) {
+        if (!checkPortQual(port, cpName, nodeId)) {
+            return false;
+        }
+        if (Direction.Bidirectional.getIntValue() == port.getPortDirection().getIntValue() ^ bidirectional) {
+        // (a ^ b) makes more sense than (!a && b) here since it can also work for unidirectional links
+            LOG.error(PortMappingUtils.PORTDIR_ERROR_LOGMSG + PortMappingUtils.CANNOT_AS_LCP_LOGMSG,
+                nodeId, port.getPortName(), cpName);
+            return false;
+        }
+        return true;
+    }
+
     private NodeInfo createNodeInfo(Info deviceInfo) {
 
         if (deviceInfo.getNodeType() == null) {

diff --git a/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingVersion710.java b/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingVersion710.java
index ebaffd1696fcdb11c7e19d6c738fd4b59cd3be8a..452889c64702665ac6c9f96ec81ae14ff5897053 100644 (file)
--- a/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingVersion710.java
+++ b/common/src/main/java/org/opendaylight/transportpce/common/mapping/PortMappingVersion710.java
@@ -18,6 +18,7 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.stream.Collectors;
 import org.eclipse.jdt.annotation.NonNull;
@@ -113,6 +114,7 @@ public class PortMappingVersion710 {
 
     private static final Logger LOG = LoggerFactory.getLogger(PortMappingVersion710.class);
     private static final Map<Direction, String> SUFFIX;
+    private static final Set<Integer> TXRX_SET = Set.of(Direction.Tx.getIntValue(), Direction.Rx.getIntValue());
 
     private final DataBroker dataBroker;
     private final DeviceTransactionManager deviceTransactionManager;
@@ -470,12 +472,9 @@ public class PortMappingVersion710 {
     }
 
     private boolean checkPartnerPort(String circuitPackName, Ports port1, Ports port2) {
-        return (checkPartnerPortNoDir(circuitPackName, port1, port2)
-            && ((Direction.Rx.getIntValue() == port1.getPortDirection().getIntValue()
-                    && Direction.Tx.getIntValue() == port2.getPortDirection().getIntValue())
-                ||
-                (Direction.Tx.getIntValue() == port1.getPortDirection().getIntValue()
-                    && Direction.Rx.getIntValue() == port2.getPortDirection().getIntValue())));
+        return checkPartnerPortNoDir(circuitPackName, port1, port2)
+            && Set.of(port1.getPortDirection().getIntValue(), port2.getPortDirection().getIntValue())
+                .equals(TXRX_SET);
     }
 
 
@@ -535,17 +534,11 @@ public class PortMappingVersion710 {
                 Collections.sort(portList, new SortPort710ByName());
                 int portIndex = 1;
                 for (Ports port : portList) {
-                    String currentKey = circuitPackName + "-" + port.getPortName();
-                    if (port.getPortQual() == null) {
-                        continue;
-                    }
-
-                    if (PortQual.RoadmExternal.getIntValue() != port.getPortQual().getIntValue()) {
-                        LOG.debug(PortMappingUtils.PORT_NOT_RDMEXT_LOGMSG + PortMappingUtils.CANNOT_AS_LCP_LOGMSG,
-                            nodeId, port.getPortName(), circuitPackName);
+                    if (!checkPortQual(port, circuitPackName, nodeId)) {
                         continue;
                     }
 
+                    String currentKey = circuitPackName + "-" + port.getPortName();
                     if (keys.contains(currentKey)) {
                         LOG.debug(PortMappingUtils.PORT_ALREADY_HANDLED_LOGMSG + PortMappingUtils.CANNOT_AS_LCP_LOGMSG,
                             nodeId, port.getPortName(), circuitPackName);
@@ -1284,17 +1277,7 @@ public class PortMappingVersion710 {
                         return false;
                     }
                     Ports port = portObject.get();
-                    if (port.getPortQual() == null) {
-                        continue;
-                    }
-                    if (PortQual.RoadmExternal.getIntValue() != port.getPortQual().getIntValue()) {
-                        LOG.error(PortMappingUtils.CANNOT_CREATE_LCP_LOGMSG + PortMappingUtils.PORTQUAL_ERROR_LOGMSG,
-                            nodeId, port.getPortName(), cp1Name);
-                        continue;
-                    }
-                    if (Direction.Bidirectional.getIntValue() != port.getPortDirection().getIntValue()) {
-                        LOG.error(PortMappingUtils.CANNOT_CREATE_LCP_LOGMSG + PortMappingUtils.PORTDIR_ERROR_LOGMSG,
-                            nodeId, port.getPortName(), cp1Name);
+                    if (!checkTtpPort(port, cp1Name, nodeId, true)) {
                         continue;
                     }
 
@@ -1332,18 +1315,11 @@ public class PortMappingVersion710 {
                     }
 
                     Ports port1 = port1Object.get();
-                    Ports port2 = port2Object.get();
-                    if (port1.getPortQual() == null || port2.getPortQual() == null) {
+                    if (!checkTtpPort(port1, cp1Name, nodeId, false)) {
                         continue;
                     }
-                    if (PortQual.RoadmExternal.getIntValue() != port1.getPortQual().getIntValue()) {
-                        LOG.error(PortMappingUtils.CANNOT_CREATE_LCP_LOGMSG + PortMappingUtils.PORTQUAL_ERROR_LOGMSG,
-                            nodeId, port1.getPortName(), cp1Name);
-                        continue;
-                    }
-                    if (PortQual.RoadmExternal.getIntValue() != port2.getPortQual().getIntValue()) {
-                        LOG.error(PortMappingUtils.CANNOT_CREATE_LCP_LOGMSG + PortMappingUtils.PORTQUAL_ERROR_LOGMSG,
-                            nodeId, port2.getPortName(), cp2Name);
+                    Ports port2 = port2Object.get();
+                    if (!checkTtpPort(port2, cp2Name, nodeId, false)) {
                         continue;
                     }
 
@@ -1378,6 +1354,32 @@ public class PortMappingVersion710 {
         return true;
     }
 
+    private boolean checkPortQual(Ports port, String cpName, String nodeId) {
+        if (port.getPortQual() == null) {
+            return false;
+        }
+        if (PortQual.RoadmExternal.getIntValue() != port.getPortQual().getIntValue()) {
+            //used to be LOG.error when called from createTtpPortMapping
+            LOG.debug(PortMappingUtils.PORT_NOT_RDMEXT_LOGMSG + PortMappingUtils.CANNOT_AS_LCP_LOGMSG,
+                nodeId, port.getPortName(), cpName);
+            return false;
+        }
+        return true;
+    }
+
+    private boolean checkTtpPort(Ports port, String cpName, String nodeId, boolean bidirectional) {
+        if (!checkPortQual(port, cpName, nodeId)) {
+            return false;
+        }
+        if (Direction.Bidirectional.getIntValue() == port.getPortDirection().getIntValue() ^ bidirectional) {
+        // (a ^ b) makes more sense than (!a && b) here since it can also work for unidirectional links
+            LOG.error(PortMappingUtils.PORTDIR_ERROR_LOGMSG + PortMappingUtils.CANNOT_AS_LCP_LOGMSG,
+                nodeId, port.getPortName(), cpName);
+            return false;
+        }
+        return true;
+    }
+
     private NodeInfo createNodeInfo(Info deviceInfo) {
 
         if (deviceInfo.getNodeType() == null) {