From c6c7b5fca2580883621d366573e20eefb9ea06e1 Mon Sep 17 00:00:00 2001 From: "guillaume.lambert" Date: Mon, 7 Feb 2022 13:01:30 +0100 Subject: [PATCH] Refactor PCE network analyzer PceOtnNode step 3 - make class static Maps and Lists final - rename some variables - rationalize conditions check & dedicated methods - use && precendence over || to remove some useless parenthesis - add a few intermediate functions and variables - add a few TODO comments on remaining redundant checks code JIRA: TRNSPRTPCE-572 Signed-off-by: guillaume.lambert Change-Id: I9fec81f36c4dd83b8000d337e84200498b960955 --- .../pce/networkanalyzer/PceOtnNode.java | 170 ++++++++---------- 1 file changed, 79 insertions(+), 91 deletions(-) diff --git a/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOtnNode.java b/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOtnNode.java index 4cd7c9437..f7cfc4a5b 100644 --- a/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOtnNode.java +++ b/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOtnNode.java @@ -36,6 +36,8 @@ import org.opendaylight.yang.gen.v1.http.org.openroadm.port.types.rev200327.If10 import org.opendaylight.yang.gen.v1.http.org.openroadm.port.types.rev200327.If1GEODU0; import org.opendaylight.yang.gen.v1.http.org.openroadm.port.types.rev200327.IfOCHOTU4ODU4; import org.opendaylight.yang.gen.v1.http.org.openroadm.port.types.rev200327.IfOtsiOtsigroup; +import org.opendaylight.yang.gen.v1.http.org.openroadm.port.types.rev200327.SupportedIfCapability; +import org.opendaylight.yang.gen.v1.http.org.openroadm.xponder.rev200529.xpdr.otn.tp.attributes.OdtuTpnPool; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.network.rev180226.NodeId; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.network.rev180226.networks.network.Node; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.network.topology.rev180226.TpId; @@ -45,31 +47,32 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class PceOtnNode implements PceNode { - /* Logging. */ - private static final Logger LOG = LoggerFactory.getLogger(PceOtnNode.class); ////////////////////////// OTN NODES /////////////////////////// /* * For This Class the node passed shall be at the otn-openroadm Layer */ - private static List serviceTypeOduList = List.of( + private static final Logger LOG = LoggerFactory.getLogger(PceOtnNode.class); + private static final List SERVICE_TYPE_ODU_LIST = List.of( StringConstants.SERVICE_TYPE_ODU4, StringConstants.SERVICE_TYPE_ODUC4, StringConstants.SERVICE_TYPE_ODUC3, StringConstants.SERVICE_TYPE_ODUC2); - private static Map> - serviceTypeEthClassMap = Map.of( + private static final List VALID_NODETYPES_LIST = List.of( + OpenroadmNodeType.MUXPDR, + OpenroadmNodeType.SWITCH, + OpenroadmNodeType.TPDR); + private static final Map> SERVICE_TYPE_ETH_CLASS_MAP = Map.of( StringConstants.SERVICE_TYPE_1GE, If1GEODU0.class, StringConstants.SERVICE_TYPE_10GE, If10GEODU2e.class, StringConstants.SERVICE_TYPE_100GE_T, If100GEODU4.class, StringConstants.SERVICE_TYPE_100GE_M, If100GEODU4.class, StringConstants.SERVICE_TYPE_100GE_S, If100GEODU4.class); - private static Map serviceTypeEthTsNbMap = Map.of( + private static final Map SERVICE_TYPE_ETH_TS_NB_MAP = Map.of( StringConstants.SERVICE_TYPE_1GE, 1, StringConstants.SERVICE_TYPE_10GE, 10, StringConstants.SERVICE_TYPE_100GE_M, 20); - private static Map serviceTypeEthOduStringMap = Map.of( + private static final Map SERVICE_TYPE_ETH_ODU_STRING_MAP = Map.of( StringConstants.SERVICE_TYPE_1GE, "ODU0", StringConstants.SERVICE_TYPE_10GE, "ODU2e", StringConstants.SERVICE_TYPE_100GE_M, "ODU4"); @@ -125,8 +128,7 @@ public class PceOtnNode implements PceNode { this.tpAvailableTribSlot.clear(); checkAvailableTribSlot(); this.clientPort = clientPort; - if (node == null || nodeId == null || nodeType != OpenroadmNodeType.MUXPDR - && nodeType != OpenroadmNodeType.SWITCH && nodeType != OpenroadmNodeType.TPDR) { + if (node == null || nodeId == null || nodeType == null || !VALID_NODETYPES_LIST.contains(nodeType)) { LOG.error("PceOtnNode: one of parameters is not populated : nodeId, node type"); this.valid = false; } @@ -163,7 +165,7 @@ public class PceOtnNode implements PceNode { continue; } TerminationPoint1 ontTp1 = tp.augmentation(TerminationPoint1.class); - if (serviceTypeOduList.contains(this.otnServiceType) + if (SERVICE_TYPE_ODU_LIST.contains(this.otnServiceType) || StringConstants.SERVICE_TYPE_100GE_S.equals(this.otnServiceType)) { // TODO verify the capability of network port to support ODU4 CTP interface creation if (!checkTpForOdtuTermination(ontTp1)) { @@ -171,11 +173,12 @@ public class PceOtnNode implements PceNode { tp.getTpId().getValue(), node.getNodeId().getValue()); continue; } - } else if (serviceTypeEthTsNbMap.containsKey(this.otnServiceType)) { - if (!checkOdtuTTPforLoOduCreation(ontTp1, serviceTypeEthTsNbMap.get(this.otnServiceType))) { + } else if (SERVICE_TYPE_ETH_TS_NB_MAP.containsKey(this.otnServiceType)) { + if (!checkOdtuTTPforLoOduCreation( + ontTp1, SERVICE_TYPE_ETH_TS_NB_MAP.get(this.otnServiceType))) { LOG.error("TP {} of {} does not allow {} termination creation", tp.getTpId().getValue(), - serviceTypeEthOduStringMap.get(this.otnServiceType), + SERVICE_TYPE_ETH_ODU_STRING_MAP.get(this.otnServiceType), node.getNodeId().getValue()); continue; } @@ -190,7 +193,7 @@ public class PceOtnNode implements PceNode { break; case XPONDERCLIENT: - if (serviceTypeEthClassMap.containsKey(otnServiceType) + if (SERVICE_TYPE_ETH_CLASS_MAP.containsKey(otnServiceType) && !StringConstants.SERVICE_TYPE_100GE_T.equals(this.otnServiceType)) { // TODO should we really exclude SERVICE_TYPE_100GE_T ? if (tp.augmentation(TerminationPoint1.class) == null) { @@ -210,21 +213,20 @@ public class PceOtnNode implements PceNode { LOG.debug("unsupported ocn TP type {}", ocnTp1.getTpType()); } } - this.valid = serviceTypeOduList.contains(this.otnServiceType) - || (serviceTypeEthTsNbMap.containsKey(this.otnServiceType) - && ((mode.equals("AZ") && checkSwPool(availableXpdrClientTps, availableXpdrNWTps, 1, 1)) - || (mode.equals("intermediate") && checkSwPool(null, availableXpdrNWTps, 0, 2))) - ) - || (StringConstants.SERVICE_TYPE_100GE_S.equals(this.otnServiceType) - && ((mode.equals("AZ") && checkSwPool(availableXpdrClientTps, availableXpdrNWTps, 1, 1))) - || (mode.equals("intermediate") && checkSwPool(availableXpdrClientTps, availableXpdrNWTps, 0, 2)) - ); - //TODO checks very similar to isIntermediate and isAz methods - they should be mutualized - // perhaps even with isOtnServiceTypeValid method + this.valid = SERVICE_TYPE_ODU_LIST.contains(this.otnServiceType) + || SERVICE_TYPE_ETH_TS_NB_MAP.containsKey(this.otnServiceType) + && isAzOrIntermediateAvl(mode, null, availableXpdrClientTps, availableXpdrNWTps) + || StringConstants.SERVICE_TYPE_100GE_S.equals(this.otnServiceType) + && isAzOrIntermediateAvl(mode, availableXpdrClientTps, availableXpdrClientTps, availableXpdrNWTps); + //TODO very similar to isOtnServiceTypeValid method + // check whether the different treatment for SERVICE_TYPE_100GE_S here is appropriate or not } private boolean checkSwPool(List clientTps, List netwTps, int nbClient, int nbNetw) { - if (clientTps != null && netwTps != null && nbClient == 1 && nbNetw == 1) { + if (netwTps == null) { + return false; + } + if (clientTps != null && nbClient == 1 && nbNetw == 1) { clientTps.sort(Comparator.comparing(TpId::getValue)); netwTps.sort(Comparator.comparing(TpId::getValue)); for (TpId nwTp : netwTps) { @@ -236,18 +238,20 @@ public class PceOtnNode implements PceNode { usableXpdrClientTps.add(clTp); usableXpdrNWTps.add(nwTp); } - if (usableXpdrClientTps.size() >= nbClient && usableXpdrNWTps.size() >= nbNetw) { - if (this.clientPort == null || this.clientPort.equals(clTp.getValue())) { - clientPerNwTp.put(nwTp.getValue(), clTp.getValue()); - return true; - } + if (usableXpdrClientTps.size() >= 1 && usableXpdrNWTps.size() >= 1 + //since nbClient == 1 && nbNetw == 1... + && (this.clientPort == null || this.clientPort.equals(clTp.getValue()))) { + clientPerNwTp.put(nwTp.getValue(), clTp.getValue()); + return true; } } } } } - if (netwTps != null && nbClient == 0 && nbNetw == 2) { + if (nbClient == 0 && nbNetw == 2) { netwTps.sort(Comparator.comparing(TpId::getValue)); + //TODO compared to above, nested loops are inverted below - does it make really sense ? + // there is room to rationalize things here for (NonBlockingList nbl : new ArrayList<>(node.augmentation(Node1.class).getSwitchingPools() .nonnullOduSwitchingPools().values().stream().findFirst().get() .getNonBlockingList().values())) { @@ -255,7 +259,8 @@ public class PceOtnNode implements PceNode { if (nbl.getTpList().contains(nwTp)) { usableXpdrNWTps.add(nwTp); } - if (usableXpdrNWTps.size() >= nbNetw) { + if (usableXpdrNWTps.size() >= 2) { + //since nbClient == 0 && nbNetw == 2... return true; } } @@ -279,21 +284,19 @@ public class PceOtnNode implements PceNode { private boolean checkOdtuTTPforLoOduCreation(TerminationPoint1 ontTp1, int tsNb) { XpdrTpPortConnectionAttributes portConAttr = ontTp1.getXpdrTpPortConnectionAttributes(); - return portConAttr != null - && portConAttr.getTsPool() != null - && portConAttr.getTsPool().size() >= tsNb - && portConAttr.getOdtuTpnPool() != null - && (portConAttr.getOdtuTpnPool().values() - .stream().findFirst().get().getOdtuType() - .equals(ODTU4TsAllocated.class) - || - portConAttr.getOdtuTpnPool().values() - .stream().findFirst().get().getOdtuType() - .equals(ODTUCnTs.class)) - && !portConAttr.getOdtuTpnPool().values() - .stream().findFirst().get().getTpnPool() - .isEmpty(); - //TODO a dedicated method for OdtuTpnPool first value would probably be welcome + if (portConAttr == null + || portConAttr.getTsPool() == null + || portConAttr.getTsPool().size() < tsNb + || portConAttr.getOdtuTpnPool() == null) { + return false; + } + return checkFirstOdtuTpn(portConAttr.getOdtuTpnPool().values().stream().findFirst().get()); + } + + private boolean checkFirstOdtuTpn(OdtuTpnPool otPool) { + return (otPool.getOdtuType().equals(ODTU4TsAllocated.class) + || otPool.getOdtuType().equals(ODTUCnTs.class)) + && !otPool.getTpnPool().isEmpty(); } private boolean checkClientTp(TerminationPoint1 ontTp1) { @@ -301,8 +304,8 @@ public class PceOtnNode implements PceNode { .values()) { LOG.debug("in checkTpForOduTermination - sic = {}", sic.getIfCapType()); // we could also check the administrative status of the tp - if (serviceTypeEthClassMap.containsKey(otnServiceType) - && sic.getIfCapType().equals(serviceTypeEthClassMap.get(otnServiceType))) { + if (SERVICE_TYPE_ETH_CLASS_MAP.containsKey(otnServiceType) + && sic.getIfCapType().equals(SERVICE_TYPE_ETH_CLASS_MAP.get(otnServiceType))) { return true; } } @@ -377,17 +380,10 @@ public class PceOtnNode implements PceNode { .collect(Collectors.toList())) { XpdrTpPortConnectionAttributes portConAttr = tp.augmentation(TerminationPoint1.class).getXpdrTpPortConnectionAttributes(); - if (portConAttr != null - && portConAttr.getOdtuTpnPool() != null - && (portConAttr.getOdtuTpnPool().values().stream().findFirst().get().getOdtuType() - .equals(ODTU4TsAllocated.class) - || portConAttr.getOdtuTpnPool().values().stream().findFirst().get().getOdtuType() - .equals(ODTUCnTs.class))) { - //TODO a dedicated method for OdtuTpnPool first value would probably be welcome - List tpnPool = - portConAttr.getOdtuTpnPool().values().stream().findFirst().get().getTpnPool(); - if (tpnPool != null) { - tpAvailableTribPort.put(tp.getTpId().getValue(), tpnPool); + if (portConAttr != null && portConAttr.getOdtuTpnPool() != null) { + OdtuTpnPool otPool = portConAttr.getOdtuTpnPool().values().stream().findFirst().get(); + if (checkFirstOdtuTpn(otPool)) { + tpAvailableTribPort.put(tp.getTpId().getValue(), otPool.getTpnPool()); } } } @@ -413,61 +409,53 @@ public class PceOtnNode implements PceNode { } public boolean isValid() { - if (node == null || nodeId == null || nodeType == null || this.getSupNetworkNodeId() == null - || this.getSupClliNodeId() == null) { + if (isNotValid(this)) { LOG.error("PceNode: one of parameters is not populated : nodeId, node type, supporting nodeId"); valid = false; } return valid; } + private boolean isNotValid(final PceOtnNode poNode) { + return poNode == null || poNode.nodeId == null || poNode.nodeType == null + || poNode.getSupNetworkNodeId() == null || poNode.getSupClliNodeId() == null; + } + public boolean isPceOtnNodeValid(final PceOtnNode pceOtnNode) { - if (pceOtnNode == null || pceOtnNode.node == null || pceOtnNode.getNodeId() == null - || pceOtnNode.nodeType == null || pceOtnNode.getSupNetworkNodeId() == null - || pceOtnNode.getSupClliNodeId() == null || pceOtnNode.otnServiceType == null) { + if (isNotValid(pceOtnNode) || pceOtnNode.otnServiceType == null) { LOG.error( "PceOtnNode: one of parameters is not populated : nodeId, node type, supporting nodeId, otnServiceType" ); return false; } - if (isNodeTypeValid(pceOtnNode)) { + if (VALID_NODETYPES_LIST.contains(pceOtnNode.nodeType)) { return isOtnServiceTypeValid(pceOtnNode); } LOG.error("PceOtnNode node type: node type is not one of MUXPDR or SWITCH or TPDR"); return false; } - private boolean isOtnServiceTypeValid(PceOtnNode pceOtnNode) { - if (pceOtnNode.modeType == null) { + private boolean isOtnServiceTypeValid(final PceOtnNode poNode) { + if (poNode.modeType == null) { return false; } //Todo refactor Strings (mode and otnServiceType ) to enums - if (pceOtnNode.otnServiceType.equals(StringConstants.SERVICE_TYPE_ODU4) - && pceOtnNode.modeType.equals("AZ")) { + if (poNode.otnServiceType.equals(StringConstants.SERVICE_TYPE_ODU4) + && poNode.modeType.equals("AZ")) { return true; } - return (pceOtnNode.otnServiceType.equals(StringConstants.SERVICE_TYPE_10GE) - || pceOtnNode.otnServiceType.equals(StringConstants.SERVICE_TYPE_1GE) - || pceOtnNode.otnServiceType.equals(StringConstants.SERVICE_TYPE_100GE_S)) - && (isAz(pceOtnNode) || isIntermediate(pceOtnNode)); - //TODO serviceTypeEthTsNbMap.containsKey(this.otnServiceType) might be more appropriate here + return (poNode.otnServiceType.equals(StringConstants.SERVICE_TYPE_10GE) + || poNode.otnServiceType.equals(StringConstants.SERVICE_TYPE_1GE) + || poNode.otnServiceType.equals(StringConstants.SERVICE_TYPE_100GE_S)) + && isAzOrIntermediateAvl(poNode.modeType, null, poNode.availableXpdrClientTps, poNode.availableXpdrNWTps); + //TODO SERVICE_TYPE_ETH_TS_NB_MAP.containsKey(this.otnServiceType) might be more appropriate here // but only SERVICE_TYPE_100GE_S is managed and not SERVICE_TYPE_100GE_M and _T } - private boolean isIntermediate(PceOtnNode pceOtnNode) { - return pceOtnNode.modeType.equals("intermediate") - && checkSwPool(null, pceOtnNode.availableXpdrNWTps, 0, 2); - } - - private boolean isAz(PceOtnNode pceOtnNode) { - return pceOtnNode.modeType.equals("AZ") - && checkSwPool(pceOtnNode.availableXpdrClientTps, pceOtnNode.availableXpdrNWTps, 1, 1); - } - - private boolean isNodeTypeValid(final PceOtnNode pceOtnNode) { - return pceOtnNode.nodeType == OpenroadmNodeType.MUXPDR - || pceOtnNode.nodeType == OpenroadmNodeType.SWITCH - || pceOtnNode.nodeType == OpenroadmNodeType.TPDR; + private boolean isAzOrIntermediateAvl( + String mdType, List clientTps0, List clientTps, List netwTps) { + return mdType.equals("intermediate") && checkSwPool(clientTps0, netwTps, 0, 2) + || mdType.equals("AZ") && checkSwPool(clientTps, netwTps, 1, 1); } @Override -- 2.36.6