From 182376521d1e85e82b633f541e5166d9c3a974f8 Mon Sep 17 00:00:00 2001 From: "guillaume.lambert" Date: Wed, 29 Sep 2021 10:57:03 +0200 Subject: [PATCH] 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 Change-Id: I625598fd935c296f59c9ad42966d5353c21a59d8 --- .../common/mapping/PortMappingUtils.java | 6 +- .../common/mapping/PortMappingVersion121.java | 75 ++++++++++--------- .../common/mapping/PortMappingVersion221.java | 72 +++++++++--------- .../common/mapping/PortMappingVersion710.java | 72 +++++++++--------- 4 files changed, 113 insertions(+), 112 deletions(-) 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 a525bb1aa..0a2ed8cc5 100644 --- 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 661a532bb..409ad5ebf 100644 --- 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 SUFFIX; + private static final Set 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 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 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 cbc4eb575..d2dcfc883 100644 --- 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 SUFFIX; + private static final Set 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 ebaffd169..452889c64 100644 --- 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 SUFFIX; + private static final Set 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) { -- 2.36.6