From 0a65ef7de52296febed7af886b01cabfa51f1acf Mon Sep 17 00:00:00 2001 From: "guillaume.lambert" Date: Tue, 15 Dec 2020 15:54:04 +0100 Subject: [PATCH] split checkPartnerPort method in PortMapping this allows to reuse the intermediate checks in several other places and to remove useless checks and to add protections. JIRA: TRNSPRTPCE-354 Signed-off-by: guillaume.lambert Change-Id: I8bdff51e7f15e516444ddb75a23e082e65ac11c4 --- .../common/mapping/PortMappingVersion121.java | 36 +++++++++++------ .../common/mapping/PortMappingVersion221.java | 40 ++++++++++++------- .../common/mapping/PortMappingVersion710.java | 40 ++++++++++++------- 3 files changed, 74 insertions(+), 42 deletions(-) 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 679dadbb1..36e2dc376 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 @@ -242,9 +242,7 @@ public class PortMappingVersion121 { // TODO PortDirection treatment here is similar to the one in createPpPortMapping. // Some code alignment must be considered. - if (port.getPartnerPort() == null - || port.getPartnerPort().getCircuitPackName() == null - || port.getPartnerPort().getPortName() == null) { + if (!checkPartnerPortNotNull(port)) { LOG.warn("Error in the configuration of port {} of {} for {}", port.getPortName(), circuitPackName, nodeId); continue; @@ -330,12 +328,26 @@ public class PortMappingVersion121 { return true; } - private boolean checkPartnerPort(String circuitPackName, Ports port1, Ports port2) { - if (port2.getPartnerPort() == null - || port2.getPartnerPort().getCircuitPackName() == null - || port2.getPartnerPort().getPortName() == null + private boolean checkPartnerPortNotNull(Ports port) { + if (port.getPartnerPort() == null + || port.getPartnerPort().getCircuitPackName() == null + || port.getPartnerPort().getPortName() == null) { + return false; + } + return true; + } + + private boolean checkPartnerPortNoDir(String circuitPackName, Ports port1, Ports port2) { + if (!checkPartnerPortNotNull(port2) || !port2.getPartnerPort().getCircuitPackName().equals(circuitPackName) - || !port2.getPartnerPort().getPortName().equals(port1.getPortName()) + || !port2.getPartnerPort().getPortName().equals(port1.getPortName())) { + return false; + } + return true; + } + + private boolean checkPartnerPort(String circuitPackName, Ports port1, Ports port2) { + if (!checkPartnerPortNoDir(circuitPackName, port1, port2) || ((Direction.Rx.getIntValue() != port1.getPortDirection().getIntValue() || Direction.Tx.getIntValue() != port2.getPortDirection().getIntValue()) && @@ -429,8 +441,8 @@ public class PortMappingVersion121 { case Rx: case Tx: - if (port.getPartnerPort() == null) { - LOG.info("{} : port {} on {} is unidirectional but has no partnerPort" + if (!checkPartnerPortNotNull(port)) { + LOG.info("{} : port {} on {} is unidirectional but has no valid partnerPort" + " - cannot assign logicalConnectionPoint.", nodeId, port.getPortName(), circuitPackName); continue; @@ -823,8 +835,8 @@ public class PortMappingVersion121 { port2.getPortName(), cp2Name, port1.getPortName(), cp1Name); continue; } - // TODO this second checkPartnerPort call has overlap checkings with the first one (Directions) - if (!checkPartnerPort(cp2Name, port2, port1)) { + // Directions checks are the same for cp1 and cp2, no need to check them twice. + if (!checkPartnerPortNoDir(cp2Name, port2, port1)) { LOG.error("port {} on {} is not a correct partner port of {} on {}", port1.getPortName(), cp1Name, port2.getPortName(), cp2Name); continue; 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 d7b4c1bdd..e6380d09e 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 @@ -268,9 +268,7 @@ public class PortMappingVersion221 { // TODO PortDirection treatment here is similar to the one in createPpPortMapping. // Some code alignment must be considered. - if (port.getPartnerPort() == null - || port.getPartnerPort().getCircuitPackName() == null - || port.getPartnerPort().getPortName() == null) { + if (!checkPartnerPortNotNull(port)) { LOG.warn("Error in the configuration of port {} of {} for {}", port.getPortName(), circuitPackName, nodeId); continue; @@ -397,9 +395,7 @@ public class PortMappingVersion221 { // TODO PortDirection treatment here is similar to the one in createPpPortMapping. // Some code alignment must be considered. - if (port.getPartnerPort() == null - || port.getPartnerPort().getCircuitPackName() == null - || port.getPartnerPort().getPortName() == null) { + if (!checkPartnerPortNotNull(port)) { LOG.warn("Error in the configuration of port {} of {} for {}", port.getPortName(), circuitPackName, nodeId); continue; @@ -531,12 +527,26 @@ public class PortMappingVersion221 { return true; } - private boolean checkPartnerPort(String circuitPackName, Ports port1, Ports port2) { - if (port2.getPartnerPort() == null - || port2.getPartnerPort().getCircuitPackName() == null - || port2.getPartnerPort().getPortName() == null + private boolean checkPartnerPortNotNull(Ports port) { + if (port.getPartnerPort() == null + || port.getPartnerPort().getCircuitPackName() == null + || port.getPartnerPort().getPortName() == null) { + return false; + } + return true; + } + + private boolean checkPartnerPortNoDir(String circuitPackName, Ports port1, Ports port2) { + if (!checkPartnerPortNotNull(port2) || !port2.getPartnerPort().getCircuitPackName().equals(circuitPackName) - || !port2.getPartnerPort().getPortName().equals(port1.getPortName()) + || !port2.getPartnerPort().getPortName().equals(port1.getPortName())) { + return false; + } + return true; + } + + private boolean checkPartnerPort(String circuitPackName, Ports port1, Ports port2) { + if (!checkPartnerPortNoDir(circuitPackName, port1, port2) || ((Direction.Rx.getIntValue() != port1.getPortDirection().getIntValue() || Direction.Tx.getIntValue() != port2.getPortDirection().getIntValue()) && @@ -628,8 +638,8 @@ public class PortMappingVersion221 { case Rx: case Tx: - if (port.getPartnerPort() == null) { - LOG.info("{} : port {} on {} is unidirectional but has no partnerPort" + if (!checkPartnerPortNotNull(port)) { + LOG.info("{} : port {} on {} is unidirectional but has no valid partnerPort" + " - cannot assign logicalConnectionPoint.", nodeId, port.getPortName(), circuitPackName); continue; @@ -1135,8 +1145,8 @@ public class PortMappingVersion221 { port2.getPortName(), cp2Name, port1.getPortName(), cp1Name); continue; } - // TODO this second checkPartnerPort call has overlap checkings with the first one (Directions) - if (!checkPartnerPort(cp2Name, port2, port1)) { + // Directions checks are the same for cp1 and cp2, no need to check them twice. + if (!checkPartnerPortNoDir(cp2Name, port2, port1)) { LOG.error("port {} on {} is not a correct partner port of {} on {}", port1.getPortName(), cp1Name, port2.getPortName(), cp2Name); continue; 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 55a689b46..ff115bfd2 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 @@ -267,9 +267,7 @@ public class PortMappingVersion710 { // TODO PortDirection treatment here is similar to the one in createPpPortMapping. // Some code alignment must be considered. - if (port.getPartnerPort() == null - || port.getPartnerPort().getCircuitPackName() == null - || port.getPartnerPort().getPortName() == null) { + if (!checkPartnerPortNotNull(port)) { LOG.warn("Error in the configuration of port {} of {} for {}", port.getPortName(), circuitPackName, nodeId); continue; @@ -396,9 +394,7 @@ public class PortMappingVersion710 { // TODO PortDirection treatment here is similar to the one in createPpPortMapping. // Some code alignment must be considered. - if (port.getPartnerPort() == null - || port.getPartnerPort().getCircuitPackName() == null - || port.getPartnerPort().getPortName() == null) { + if (!checkPartnerPortNotNull(port)) { LOG.warn("Error in the configuration of port {} of {} for {}", port.getPortName(), circuitPackName, nodeId); continue; @@ -531,12 +527,26 @@ public class PortMappingVersion710 { return true; } - private boolean checkPartnerPort(String circuitPackName, Ports port1, Ports port2) { - if (port2.getPartnerPort() == null - || port2.getPartnerPort().getCircuitPackName() == null - || port2.getPartnerPort().getPortName() == null + private boolean checkPartnerPortNotNull(Ports port) { + if (port.getPartnerPort() == null + || port.getPartnerPort().getCircuitPackName() == null + || port.getPartnerPort().getPortName() == null) { + return false; + } + return true; + } + + private boolean checkPartnerPortNoDir(String circuitPackName, Ports port1, Ports port2) { + if (!checkPartnerPortNotNull(port2) || !port2.getPartnerPort().getCircuitPackName().equals(circuitPackName) - || !port2.getPartnerPort().getPortName().equals(port1.getPortName()) + || !port2.getPartnerPort().getPortName().equals(port1.getPortName())) { + return false; + } + return true; + } + + private boolean checkPartnerPort(String circuitPackName, Ports port1, Ports port2) { + if (!checkPartnerPortNoDir(circuitPackName, port1, port2) || ((Direction.Rx.getIntValue() != port1.getPortDirection().getIntValue() || Direction.Tx.getIntValue() != port2.getPortDirection().getIntValue()) && @@ -628,8 +638,8 @@ public class PortMappingVersion710 { case Rx: case Tx: - if (port.getPartnerPort() == null) { - LOG.info("{} : port {} on {} is unidirectional but has no partnerPort" + if (!checkPartnerPortNotNull(port)) { + LOG.info("{} : port {} on {} is unidirectional but has no valid partnerPort" + " - cannot assign logicalConnectionPoint.", nodeId, port.getPortName(), circuitPackName); continue; @@ -1176,8 +1186,8 @@ public class PortMappingVersion710 { port2.getPortName(), cp2Name, port1.getPortName(), cp1Name); continue; } - // TODO this second checkPartnerPort call has overlap checkings with the first one (Directions) - if (!checkPartnerPort(cp2Name, port2, port1)) { + // Directions checks are the same for cp1 and cp2, no need to check them twice. + if (!checkPartnerPortNoDir(cp2Name, port2, port1)) { LOG.error("port {} on {} is not a correct partner port of {} on {}", port1.getPortName(), cp1Name, port2.getPortName(), cp2Name); continue; -- 2.36.6