From bc9a08be9d7cdeb30ecffd3c82ddd656a3a23043 Mon Sep 17 00:00:00 2001 From: manuedelf Date: Fri, 9 Oct 2020 11:47:46 +0200 Subject: [PATCH] Technical debt - Fix PCE sonar issues JIRA: TRNSPRTPCE-274 Change-Id: If50936c2cf75be450212f9f568125cfb6033a0d8 Signed-off-by: manuedelf --- .../transportpce/pce/PcePathDescription.java | 74 ++++++++++++------- .../transportpce/pce/PceSendingPceRPCs.java | 4 +- .../transportpce/pce/gnpy/GnpyResult.java | 6 +- .../pce/gnpy/GnpyUtilitiesImpl.java | 31 +++++--- .../pce/graph/InAlgoPathValidator.java | 16 ++-- .../transportpce/pce/graph/PceGraphEdge.java | 1 + .../pce/networkanalyzer/MapUtils.java | 58 ++++++++------- .../pce/networkanalyzer/PceLink.java | 66 +++++++---------- .../pce/networkanalyzer/PceOpticalNode.java | 18 ++--- .../pce/networkanalyzer/PceOtnNode.java | 16 ++-- .../service/PathComputationServiceImpl.java | 9 ++- .../pce/impl/PceProviderTest.java | 5 +- tests/transportpce_tests/1.2.1/test_gnpy.py | 24 +++--- 13 files changed, 181 insertions(+), 147 deletions(-) diff --git a/pce/src/main/java/org/opendaylight/transportpce/pce/PcePathDescription.java b/pce/src/main/java/org/opendaylight/transportpce/pce/PcePathDescription.java index b06774591..351f4400b 100644 --- a/pce/src/main/java/org/opendaylight/transportpce/pce/PcePathDescription.java +++ b/pce/src/main/java/org/opendaylight/transportpce/pce/PcePathDescription.java @@ -39,7 +39,6 @@ public class PcePathDescription { private static final Logger LOG = LoggerFactory.getLogger(PcePathDescription.class); private List pathAtoZ = null; - private List pathZtoA = null; private PceResult rc; private Map allPceLinks = null; @@ -52,22 +51,45 @@ public class PcePathDescription { public PceResult buildDescriptions() { LOG.info("In buildDescriptions: AtoZ = {}", pathAtoZ); - Map atozList = new HashMap<>(); + Map atozMap = new HashMap<>(); if (pathAtoZ == null) { rc.setRC(ResponseCodes.RESPONSE_FAILED); LOG.error("In buildDescriptions: there is empty AtoZ path"); return rc; } - buildAtoZ(atozList, pathAtoZ); + buildAtoZ(atozMap, pathAtoZ); + rc.setAtoZDirection(buildAtoZDirection(atozMap).build()); + List pathZtoA = ImmutableList.copyOf(pathAtoZ).reverse(); + LOG.info("In buildDescriptions: ZtoA {}", pathZtoA); + + Map ztoaMap = new HashMap<>(); + if (pathZtoA == null) { + rc.setRC(ResponseCodes.RESPONSE_FAILED); + LOG.error("In buildDescriptions: there is empty ZtoA path"); + return rc; + } + buildZtoA(ztoaMap, pathZtoA); + rc.setZtoADirection(buildZtoADirection(ztoaMap).build()); + + return rc; + } + + /** + * Create a builder for AtoZDirection object. + * @param atozMap Map of AToZ object + * @return a builder for AtoZDirection object + */ + private AToZDirectionBuilder buildAtoZDirection(Map atozMap) { AToZDirectionBuilder atoZDirectionBldr = new AToZDirectionBuilder() .setRate(Uint32.valueOf(rc.getRate())) - .setAToZ(atozList); + .setAToZ(atozMap); if ("100GE".equals(rc.getServiceType()) || "OTU4".equals(rc.getServiceType())) { atoZDirectionBldr.setAToZWavelengthNumber(Uint32.valueOf(rc.getResultWavelength())); } else if ("10GE".equals(rc.getServiceType()) || "1GE".equals(rc.getServiceType()) || "ODU4".equals(rc.getServiceType())) { if (rc.getResultTribSlot() != null && rc.getResultTribPort() != null) { + @SuppressWarnings("unchecked") List tribSlotList = (List) rc.getResultTribSlot().values().toArray()[0]; atoZDirectionBldr.setAToZWavelengthNumber(Uint32.valueOf(0)) .setTribPortNumber(Uint16.valueOf(rc.getResultTribPort().values().toArray()[0].toString())) @@ -77,25 +99,24 @@ public class PcePathDescription { atoZDirectionBldr.setTribSlotNumber(Uint16.valueOf(0)).setTribPortNumber(Uint16.valueOf(0)); } } - rc.setAtoZDirection(atoZDirectionBldr.build()); - pathZtoA = ImmutableList.copyOf(pathAtoZ).reverse(); - LOG.info("In buildDescriptions: ZtoA {}", pathZtoA); + return atoZDirectionBldr; + } - Map ztoaList = new HashMap<>(); - if (pathZtoA == null) { - rc.setRC(ResponseCodes.RESPONSE_FAILED); - LOG.error("In buildDescriptions: there is empty ZtoA path"); - return rc; - } - buildZtoA(ztoaList, pathZtoA); + /** + * Create a builder for ZtoADirection object. + * @param ztoaMap Map of ZToA object + * @return a builder for ZtoADirection object + */ + private ZToADirectionBuilder buildZtoADirection(Map ztoaMap) { ZToADirectionBuilder ztoADirectionBldr = new ZToADirectionBuilder() .setRate(Uint32.valueOf(rc.getRate())) - .setZToA(ztoaList); + .setZToA(ztoaMap); if ("100GE".equals(rc.getServiceType()) || "OTU4".equals(rc.getServiceType())) { ztoADirectionBldr.setZToAWavelengthNumber(Uint32.valueOf(rc.getResultWavelength())); } else if ("10GE".equals(rc.getServiceType()) || "1GE".equals(rc.getServiceType()) || "ODU4".equals(rc.getServiceType())) { if (rc.getResultTribSlot() != null && rc.getResultTribPort() != null) { + @SuppressWarnings("unchecked") List tribSlotList = (List) rc.getResultTribSlot().values().toArray()[0]; ztoADirectionBldr.setZToAWavelengthNumber(Uint32.valueOf(0)) .setTribPortNumber(Uint16.valueOf(rc.getResultTribPort().values().toArray()[0].toString())) @@ -105,12 +126,13 @@ public class PcePathDescription { ztoADirectionBldr.setTribSlotNumber(Uint16.valueOf(0)).setTribPortNumber(Uint16.valueOf(0)); } } - rc.setZtoADirection(ztoADirectionBldr.build()); - - return rc; + return ztoADirectionBldr; } - private void buildAtoZ(Map atozList, List path) { + @SuppressWarnings("java:S138") + //sonar issue This method has 77 lines, which is greater than the 75 lines authorized. Split it into smaller + //ignore as it's not relevant to split it from functional point + private void buildAtoZ(Map atozMap, List path) { Integer index = 0; PceLink lastLink = null; AToZ lastResource = null; @@ -125,7 +147,7 @@ public class PcePathDescription { AToZKey clientKey = new AToZKey(index.toString()); Resource clientResource = new ResourceBuilder().setResource(stp).build(); AToZ firstResource = new AToZBuilder().setId(tpName).withKey(clientKey).setResource(clientResource).build(); - atozList.put(firstResource.key(),firstResource); + atozMap.put(firstResource.key(),firstResource); index += 1; for (PceLink pcelink : path) { String srcName = pcelink.getSourceId().getValue(); @@ -140,7 +162,7 @@ public class PcePathDescription { Resource nodeResource1 = new ResourceBuilder().setResource(sourceNode).build(); AToZ srcResource = new AToZBuilder().setId(srcName).withKey(sourceKey).setResource(nodeResource1).build(); index += 1; - atozList.put(srcResource.key(),srcResource); + atozMap.put(srcResource.key(),srcResource); // source TP tpName = pcelink.getSourceTP().toString(); @@ -153,7 +175,7 @@ public class PcePathDescription { Resource tpResource1 = new ResourceBuilder().setResource(stp).build(); AToZ stpResource = new AToZBuilder().setId(tpName).withKey(srcTPKey).setResource(tpResource1).build(); index += 1; - atozList.put(stpResource.key(),stpResource); + atozMap.put(stpResource.key(),stpResource); String linkName = pcelink.getLinkId().getValue(); // Link @@ -167,7 +189,7 @@ public class PcePathDescription { Resource nodeResource2 = new ResourceBuilder().setResource(atozLink).build(); AToZ linkResource = new AToZBuilder().setId(linkName).withKey(linkKey).setResource(nodeResource2).build(); index += 1; - atozList.put(linkResource.key(),linkResource); + atozMap.put(linkResource.key(),linkResource); String destName = pcelink.getDestId().getValue(); // target TP @@ -181,7 +203,7 @@ public class PcePathDescription { Resource tpResource2 = new ResourceBuilder().setResource(dtp).build(); AToZ ttpResource = new AToZBuilder().setId(tpName).withKey(destTPKey).setResource(tpResource2).build(); index += 1; - atozList.put(ttpResource.key(),ttpResource); + atozMap.put(ttpResource.key(),ttpResource); org.opendaylight.yang.gen.v1.http.org.transportpce.b.c._interface.pathdescription.rev200629.pce .resource.resource.resource.Node targetNode = new NodeBuilder() @@ -197,7 +219,7 @@ public class PcePathDescription { } if (lastResource != null) { - atozList.put(lastResource.key(),lastResource); + atozMap.put(lastResource.key(),lastResource); } // build Z side Client TP @@ -211,7 +233,7 @@ public class PcePathDescription { clientKey = new AToZKey(index.toString()); clientResource = new ResourceBuilder().setResource(stp).build(); lastResource = new AToZBuilder().setId(tpName).withKey(clientKey).setResource(clientResource).build(); - atozList.put(lastResource.key(),lastResource); + atozMap.put(lastResource.key(),lastResource); } diff --git a/pce/src/main/java/org/opendaylight/transportpce/pce/PceSendingPceRPCs.java b/pce/src/main/java/org/opendaylight/transportpce/pce/PceSendingPceRPCs.java index 5dc2c9c7f..52d6901bd 100644 --- a/pce/src/main/java/org/opendaylight/transportpce/pce/PceSendingPceRPCs.java +++ b/pce/src/main/java/org/opendaylight/transportpce/pce/PceSendingPceRPCs.java @@ -175,7 +175,7 @@ public class PceSendingPceRPCs { } private boolean gnpyToCheckFeasiblity(AToZDirection atoz, ZToADirection ztoa, GnpyUtilitiesImpl gnpy) - throws GnpyException, Exception { + throws GnpyException { //Call GNPy for path verification if (gnpy.verifyComputationByGnpy(atoz, ztoa, pceHardConstraints)) { @@ -187,7 +187,7 @@ public class PceSendingPceRPCs { return false; } - private void callGnpyToComputeNewPath(GnpyUtilitiesImpl gnpy) throws GnpyException, Exception { + private void callGnpyToComputeNewPath(GnpyUtilitiesImpl gnpy) throws GnpyException { //Call GNPy in the case of non feasibility LOG.info("In pceSendingPceRPC: the path is not feasible according to Gnpy"); diff --git a/pce/src/main/java/org/opendaylight/transportpce/pce/gnpy/GnpyResult.java b/pce/src/main/java/org/opendaylight/transportpce/pce/gnpy/GnpyResult.java index b40fc2255..ab127889f 100644 --- a/pce/src/main/java/org/opendaylight/transportpce/pce/gnpy/GnpyResult.java +++ b/pce/src/main/java/org/opendaylight/transportpce/pce/gnpy/GnpyResult.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Optional; +import java.util.stream.Collectors; import javax.annotation.Nonnull; import org.opendaylight.mdsal.binding.dom.codec.api.BindingNormalizedNodeSerializer; import org.opendaylight.mdsal.binding.dom.codec.spi.BindingDOMCodecServices; @@ -199,7 +200,10 @@ public class GnpyResult { } } } - Include include = new IncludeBuilder().setOrderedHops(orderedHopsList).build(); + Include include = new IncludeBuilder() + .setOrderedHops(orderedHopsList.stream() + .collect(Collectors.toMap(OrderedHops::key, orderedHops -> orderedHops))) + .build(); General general = new GeneralBuilder().setInclude(include).build(); hardConstraints = new HardConstraintsBuilder().setCoRoutingOrGeneral(general).build(); return hardConstraints; diff --git a/pce/src/main/java/org/opendaylight/transportpce/pce/gnpy/GnpyUtilitiesImpl.java b/pce/src/main/java/org/opendaylight/transportpce/pce/gnpy/GnpyUtilitiesImpl.java index 7f69ba6dc..4aad49b75 100644 --- a/pce/src/main/java/org/opendaylight/transportpce/pce/gnpy/GnpyUtilitiesImpl.java +++ b/pce/src/main/java/org/opendaylight/transportpce/pce/gnpy/GnpyUtilitiesImpl.java @@ -10,6 +10,7 @@ package org.opendaylight.transportpce.pce.gnpy; import java.util.ArrayList; import java.util.List; +import java.util.stream.Collectors; import org.opendaylight.mdsal.binding.dom.codec.spi.BindingDOMCodecServices; import org.opendaylight.transportpce.common.network.NetworkTransactionService; import org.opendaylight.transportpce.pce.constraints.PceConstraints; @@ -42,7 +43,6 @@ import org.slf4j.LoggerFactory; public class GnpyUtilitiesImpl { private static final Logger LOG = LoggerFactory.getLogger(GnpyUtilitiesImpl.class); - private NetworkTransactionService networkTransaction; private PathComputationRequestInput input; private GnpyTopoImpl gnpyTopo = null; private GnpyResult gnpyAtoZ; @@ -54,8 +54,6 @@ public class GnpyUtilitiesImpl { public GnpyUtilitiesImpl(NetworkTransactionService networkTransaction, PathComputationRequestInput input, BindingDOMCodecServices bindingDOMCodecServices) throws GnpyException { - - this.networkTransaction = networkTransaction; this.gnpyTopo = new GnpyTopoImpl(networkTransaction); this.input = input; this.gnpyAtoZ = null; @@ -65,7 +63,7 @@ public class GnpyUtilitiesImpl { } public boolean verifyComputationByGnpy(AToZDirection atoz, ZToADirection ztoa, PceConstraints pceHardConstraints) - throws GnpyException, Exception { + throws GnpyException { if (atoz == null || atoz.getAToZ() == null || ztoa == null || ztoa.getZToA() == null) { throw new GnpyException("In GnpyUtilities: the path transmitted to Gnpy is null"); @@ -81,15 +79,21 @@ public class GnpyUtilitiesImpl { return isPcePathFeasible; } - public GnpyResult gnpyResponseOneDirection(GnpyServiceImpl gnpySvc) throws GnpyException, Exception { + @SuppressWarnings("checkstyle:illegalcatch") + public GnpyResult gnpyResponseOneDirection(GnpyServiceImpl gnpySvc) throws GnpyException { requestId = Uint32.valueOf((requestId.toJava()) + 1); List pathRequestList = new ArrayList<>(gnpySvc.getPathRequest().values()); List synchronizationList = gnpySvc.getSynchronization(); // Send the computed path to GNPY tool List elementsList = new ArrayList<>(gnpyTopo.getElements().values()); List connectionsList = gnpyTopo.getConnections(); - String gnpyResponse = getGnpyResponse(elementsList, connectionsList, pathRequestList, - synchronizationList); + String gnpyResponse; + try { + gnpyResponse = getGnpyResponse(elementsList, connectionsList, pathRequestList, + synchronizationList); + } catch (Exception e) { + throw new GnpyException("Something went wrong", e); + } // Analyze the response if (gnpyResponse == null) { throw new GnpyException("In GnpyUtilities: no response from GNPy server"); @@ -100,7 +104,7 @@ public class GnpyUtilitiesImpl { } public HardConstraints askNewPathFromGnpy(PceConstraints pceHardConstraints) - throws GnpyException, Exception { + throws GnpyException { AToZDirection atoztmp = new AToZDirectionBuilder() .setRate(input.getServiceAEnd().getServiceRate()) @@ -121,12 +125,17 @@ public class GnpyUtilitiesImpl { public String getGnpyResponse(List elementsList, List connectionsList, List pathRequestList, List synchronizationList) - throws GnpyException, Exception { + throws GnpyException { GnpyApi gnpyApi = new GnpyApiBuilder() .setTopologyFile( - new TopologyFileBuilder().setElements(elementsList).setConnections(connectionsList).build()) + new TopologyFileBuilder() + .setElements(elementsList.stream().collect(Collectors.toMap(Elements::key, element -> element))) + .setConnections(connectionsList).build()) .setServiceFile( - new ServiceFileBuilder().setPathRequest(pathRequestList).build()) + new ServiceFileBuilder() + .setPathRequest(pathRequestList.stream() + .collect(Collectors.toMap(PathRequest::key, pathRequest -> pathRequest))) + .build()) .build(); InstanceIdentifier idGnpyApi = InstanceIdentifier.builder(GnpyApi.class).build(); String gnpyJson; diff --git a/pce/src/main/java/org/opendaylight/transportpce/pce/graph/InAlgoPathValidator.java b/pce/src/main/java/org/opendaylight/transportpce/pce/graph/InAlgoPathValidator.java index 39ee022de..c915a2772 100644 --- a/pce/src/main/java/org/opendaylight/transportpce/pce/graph/InAlgoPathValidator.java +++ b/pce/src/main/java/org/opendaylight/transportpce/pce/graph/InAlgoPathValidator.java @@ -17,6 +17,7 @@ import org.slf4j.LoggerFactory; public class InAlgoPathValidator implements PathValidator { /* Logging. */ private static final Logger LOG = LoggerFactory.getLogger(InAlgoPathValidator.class); + private static final String IN_CHECK_PATH_DROPPED_MSG = "in checkPath dropped {} {} "; public InAlgoPathValidator() { super(); @@ -34,35 +35,38 @@ public class InAlgoPathValidator implements PathValidator return (checkTurn(partialPath.getEdgeList().get(size - 1).link().getlinkType(), edge.link().getlinkType())); } + @SuppressWarnings("java:S1541") + //sonar issue The Cyclomatic Complexity of this method "checkTurn" is 13 which is greater than 10 authorized. + //here we have clear conditional, so for the moment no need to manage this issue private boolean checkTurn(OpenroadmLinkType prevType, OpenroadmLinkType nextType) { if (nextType == OpenroadmLinkType.ADDLINK && prevType != OpenroadmLinkType.XPONDEROUTPUT) { - LOG.debug("in checkPath dropped {} {} ", prevType, nextType); + LOG.debug(IN_CHECK_PATH_DROPPED_MSG, prevType, nextType); return false; } if (nextType == OpenroadmLinkType.EXPRESSLINK && prevType != OpenroadmLinkType.ROADMTOROADM) { - LOG.debug("in checkPath dropped {} {} ", prevType, nextType); + LOG.debug(IN_CHECK_PATH_DROPPED_MSG, prevType, nextType); return false; } if (nextType == OpenroadmLinkType.DROPLINK && prevType != OpenroadmLinkType.ROADMTOROADM) { - LOG.debug("in checkPath dropped {} {} ", prevType, nextType); + LOG.debug(IN_CHECK_PATH_DROPPED_MSG, prevType, nextType); return false; } if (nextType == OpenroadmLinkType.XPONDERINPUT && prevType != OpenroadmLinkType.DROPLINK) { - LOG.debug("in checkPath dropped {} {} ", prevType, nextType); + LOG.debug(IN_CHECK_PATH_DROPPED_MSG, prevType, nextType); return false; } if (prevType == OpenroadmLinkType.EXPRESSLINK && nextType != OpenroadmLinkType.ROADMTOROADM) { - LOG.debug("in checkPath dropped {} {} ", prevType, nextType); + LOG.debug(IN_CHECK_PATH_DROPPED_MSG, prevType, nextType); return false; } if (prevType == OpenroadmLinkType.ADDLINK && nextType != OpenroadmLinkType.ROADMTOROADM) { - LOG.debug("in checkPath dropped {} {} ", prevType, nextType); + LOG.debug(IN_CHECK_PATH_DROPPED_MSG, prevType, nextType); return false; } diff --git a/pce/src/main/java/org/opendaylight/transportpce/pce/graph/PceGraphEdge.java b/pce/src/main/java/org/opendaylight/transportpce/pce/graph/PceGraphEdge.java index 87adfcce4..6983d3f18 100644 --- a/pce/src/main/java/org/opendaylight/transportpce/pce/graph/PceGraphEdge.java +++ b/pce/src/main/java/org/opendaylight/transportpce/pce/graph/PceGraphEdge.java @@ -10,6 +10,7 @@ package org.opendaylight.transportpce.pce.graph; import org.jgrapht.graph.DefaultWeightedEdge; import org.opendaylight.transportpce.pce.networkanalyzer.PceLink; +@SuppressWarnings("serial") @edu.umd.cs.findbugs.annotations.SuppressFBWarnings( value = "SE_NO_SERIALVERSIONID", justification = "https://github.com/rzwitserloot/lombok/wiki/WHY-NOT:-serialVersionUID") diff --git a/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/MapUtils.java b/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/MapUtils.java index f394e2b02..cd3fe88e1 100644 --- a/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/MapUtils.java +++ b/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/MapUtils.java @@ -12,6 +12,7 @@ import java.util.Arrays; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.SortedMap; import java.util.TreeMap; import org.opendaylight.transportpce.common.NetworkUtils; import org.opendaylight.transportpce.pce.constraints.PceConstraints; @@ -29,6 +30,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; public final class MapUtils { + private static final String MAP_UTILS_NO_LINK_AUGMENTATION_AVAILABLE_MSG = + "MapUtils: No Link augmentation available. {}"; /* Logging. */ private static final Logger LOG = LoggerFactory.getLogger(MapUtils.class); @@ -84,33 +87,35 @@ public final class MapUtils { } public static List getSRLG(Link link) { + Span omsAttributesSpan = getOmsAttributesSpan(link); + if (omsAttributesSpan == null) { + LOG.debug("No concatenation for this link"); + return new ArrayList<>(); + } List srlgList = new ArrayList<>(); - try { - Map linkList = getOmsAttributesSpan(link).getLinkConcatenation(); - for (LinkConcatenation lc : linkList.values()) { + Map linkList = omsAttributesSpan.nonnullLinkConcatenation(); + for (LinkConcatenation lc : linkList.values()) { + if (lc != null && lc.getSRLGId() != null) { srlgList.add(lc.getSRLGId().toJava()); } - } catch (NullPointerException e) { - LOG.debug("No concatenation for this link"); } return srlgList; } public static List getSRLGfromLink(Link link) { - List srlgList = new ArrayList<>(); - org.opendaylight.yang.gen.v1.http.org.openroadm.common.network.rev181130.Link1 linkC = - link.augmentation(org.opendaylight.yang.gen.v1.http.org.openroadm.common.network.rev181130.Link1.class); + org.opendaylight.yang.gen.v1.http.org.openroadm.common.network.rev181130.Link1 linkC = link + .augmentation(org.opendaylight.yang.gen.v1.http.org.openroadm.common.network.rev181130.Link1.class); if (linkC == null) { - LOG.error("MapUtils: No Link augmentation available. {}", link.getLinkId().getValue()); - - } else { - try { - for (org.opendaylight.yang.gen.v1.http.org.openroadm.common.network.rev181130.networks.network.link - .LinkConcatenation lc : linkC.nonnullLinkConcatenation().values()) { - srlgList.add(lc.getSRLGId().toJava()); - } - } catch (NullPointerException e) { - LOG.debug("No concatenation for this link"); + LOG.error(MAP_UTILS_NO_LINK_AUGMENTATION_AVAILABLE_MSG, link.getLinkId().getValue()); + return new ArrayList<>(); + } + List srlgList = new ArrayList<>(); + for (org.opendaylight.yang.gen.v1.http.org.openroadm.common.network.rev181130 + .networks.network.link.LinkConcatenation lc : linkC.nonnullLinkConcatenation().values()) { + if (lc != null && lc.getSRLGId() != null) { + srlgList.add(lc.getSRLGId().toJava()); + } else { + LOG.debug("No concatenation or SLRG id for this link"); } } return srlgList; @@ -134,7 +139,7 @@ public final class MapUtils { return null; } - public static TreeMap getAllSupNode(Node node) { + public static SortedMap getAllSupNode(Node node) { TreeMap allSupNodes = new TreeMap<>(); for (SupportingNode supnode : node.nonnullSupportingNode().values()) { allSupNodes.put(supnode.getNetworkRef().getValue(), @@ -188,7 +193,7 @@ public final class MapUtils { // ID and type link1 = link.augmentation(Link1.class); if (link1 == null) { - LOG.error("MapUtils: No Link augmentation available. {}", link.getLinkId().getValue()); + LOG.error(MAP_UTILS_NO_LINK_AUGMENTATION_AVAILABLE_MSG, link.getLinkId().getValue()); return null; } @@ -203,21 +208,18 @@ public final class MapUtils { public static Span getOmsAttributesSpan(Link link) { org.opendaylight.yang.gen.v1.http.org.openroadm.network.topology.rev181130.Link1 link1 = null; - Span tempSpan = null; link1 = link.augmentation(org.opendaylight.yang.gen.v1.http.org.openroadm.network.topology.rev181130.Link1.class); if (link1 == null) { - LOG.error("MapUtils: No Link augmentation available. {}", link.getLinkId().getValue()); - } - try { - tempSpan = link1.getOMSAttributes().getSpan(); + LOG.error(MAP_UTILS_NO_LINK_AUGMENTATION_AVAILABLE_MSG, link.getLinkId().getValue()); + return null; } - catch (NullPointerException e) { + if (link1.getOMSAttributes() == null) { LOG.error("MapUtils: No Link getOMSAttributes available. {}", link.getLinkId().getValue()); + return null; } - - return tempSpan; + return link1.getOMSAttributes().getSpan(); } public static LinkId extractOppositeLink(Link link) { diff --git a/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceLink.java b/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceLink.java index cf303055b..a38682dec 100644 --- a/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceLink.java +++ b/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceLink.java @@ -16,7 +16,6 @@ import java.util.Collection; import java.util.Iterator; import java.util.List; import java.util.Map; -import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yang.gen.v1.http.org.openroadm.common.network.rev181130.Link1; import org.opendaylight.yang.gen.v1.http.org.openroadm.link.rev181130.span.attributes.LinkConcatenation; import org.opendaylight.yang.gen.v1.http.org.openroadm.link.rev181130.span.attributes.LinkConcatenation.FiberType; @@ -30,6 +29,7 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.network.top import org.slf4j.Logger; import org.slf4j.LoggerFactory; +@SuppressWarnings("serial") @edu.umd.cs.findbugs.annotations.SuppressFBWarnings( value = "SE_NO_SERIALVERSIONID", justification = "https://github.com/rzwitserloot/lombok/wiki/WHY-NOT:-serialVersionUID") @@ -43,7 +43,6 @@ public class PceLink implements Serializable { */ double weight = 0; private boolean isValid = true; - private boolean isOtnValid = true; // this member is for XPONDER INPUT/OUTPUT links. // it keeps name of client corresponding to NETWORK TP @@ -65,7 +64,8 @@ public class PceLink implements Serializable { private final List srlgList; private final double osnr; private final transient Span omsAttributesSpan; - private static final double CELERITY = 2.99792458 * 1e5; //meter per ms + //meter per ms + private static final double CELERITY = 2.99792458 * 1e5; private static final double NOISE_MASK_A = 0.571429; private static final double NOISE_MASK_B = 39.285714; private static final double UPPER_BOUND_OSNR = 33; @@ -110,7 +110,8 @@ public class PceLink implements Serializable { this.omsAttributesSpan = null; this.srlgList = null; this.latency = 0L; - this.osnr = 100L; //infinite OSNR in DB + //infinite OSNR in DB + this.osnr = 100L; this.availableBandwidth = 0L; this.usedBandwidth = 0L; } @@ -129,30 +130,26 @@ public class PceLink implements Serializable { //Compute the link latency : if the latency is not defined, the latency is computed from the omsAttributesSpan private Long calcLatency(Link link) { - Link1 link1 = null; - Long tmplatency; - link1 = link.augmentation(Link1.class); + Link1 link1 = link.augmentation(Link1.class); if (link1.getLinkLatency() != null) { - tmplatency = link1.getLinkLatency().toJava(); - return tmplatency; + return link1.getLinkLatency().toJava(); } - - try { - double tmp = 0; - @NonNull - Map linkConcatenationMap = - this.omsAttributesSpan.nonnullLinkConcatenation(); - for (Map.Entry entry : linkConcatenationMap.entrySet()) { - //Length is expressed in meter and latency is expressed in ms according to OpenROADM MSA - tmp += entry.getValue().getSRLGLength().toJava() / CELERITY; - LOG.info("In PceLink: The latency of link {} == {}",link.getLinkId(),tmp); + if (this.omsAttributesSpan == null) { + return 1L; + } + double tmp = 0; + Map linkConcatenationMap = this.omsAttributesSpan + .nonnullLinkConcatenation(); + for (Map.Entry entry : linkConcatenationMap.entrySet()) { + // Length is expressed in meter and latency is expressed in ms according to OpenROADM MSA + if (entry == null || entry.getValue() == null || entry.getValue().getSRLGLength() == null) { + LOG.debug("In PceLink: cannot compute the latency for the link {}", link.getLinkId().getValue()); + return 1L; } - tmplatency = (long) Math.ceil(tmp); - } catch (NullPointerException e) { - LOG.debug("In PceLink: cannot compute the latency for the link {}",link.getLinkId().getValue()); - tmplatency = 1L; + tmp += entry.getValue().getSRLGLength().toJava() / CELERITY; + LOG.info("In PceLink: The latency of link {} == {}", link.getLinkId(), tmp); } - return tmplatency; + return (long) Math.ceil(tmp); } //Compute the OSNR of a span @@ -174,7 +171,8 @@ public class PceLink implements Serializable { double pout = retrievePower(linkConcatenationiterator.next().getFiberType()); // span loss (dB) double spanLoss = this.omsAttributesSpan.getSpanlossCurrent().getValue().doubleValue(); - double pin = pout - spanLoss; // power on the input of the current ROADM (dBm) + // power on the input of the current ROADM (dBm) + double pin = pout - spanLoss; double spanOsnrDb = NOISE_MASK_A * pin + NOISE_MASK_B; if (spanOsnrDb > UPPER_BOUND_OSNR) { spanOsnrDb = UPPER_BOUND_OSNR; @@ -286,19 +284,7 @@ public class PceLink implements Serializable { isValid = false; LOG.error("PceLink: No Link type or opposite link is available. Link is ignored {}", linkId); } - if ((this.sourceId == null) || (this.destId == null) || (this.sourceTP == null) || (this.destTP == null)) { - isValid = false; - LOG.error("PceLink: No Link source or destination is available. Link is ignored {}", linkId); - } - if ((this.sourceNetworkSupNodeId.equals("")) || (this.destNetworkSupNodeId.equals(""))) { - isValid = false; - LOG.error("PceLink: No Link source SuppNodeID or destination SuppNodeID is available. Link is ignored {}", - linkId); - } - if ((this.sourceCLLI.equals("")) || (this.destCLLI.equals(""))) { - isValid = false; - LOG.error("PceLink: No Link source CLLI or destination CLLI is available. Link is ignored {}", linkId); - } + isValid = checkParams(); if ((this.omsAttributesSpan == null) && (this.linkType == OpenroadmLinkType.ROADMTOROADM)) { isValid = false; LOG.error("PceLink: Error reading Span for OMS link. Link is ignored {}", linkId); @@ -365,6 +351,10 @@ public class PceLink implements Serializable { linkId, serviceType); } + return checkParams(); + } + + private boolean checkParams() { if ((this.linkId == null) || (this.linkType == null) || (this.oppositeLink == null)) { LOG.error("PceLink: No Link type or opposite link is available. Link is ignored {}", linkId); return false; diff --git a/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOpticalNode.java b/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOpticalNode.java index a9199aec3..80574e4cf 100644 --- a/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOpticalNode.java +++ b/pce/src/main/java/org/opendaylight/transportpce/pce/networkanalyzer/PceOpticalNode.java @@ -71,8 +71,8 @@ public class PceOpticalNode implements PceNode { this.node.augmentation(org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang .ietf.network.topology.rev180226.Node1.class); List allTps = new ArrayList(nodeTp.getTerminationPoint().values()); - if (allTps == null) { + .node.TerminationPoint> allTps = new ArrayList<>(nodeTp.nonnullTerminationPoint().values()); + if (allTps.isEmpty()) { LOG.error("initSrgTpList: ROADM TerminationPoint list is empty for node {}", this); this.valid = false; return; @@ -100,7 +100,7 @@ public class PceOpticalNode implements PceNode { LOG.info("initSrgTpList: SRG-PP tp = {} found", tp.getTpId().getValue()); try { List usedWavelengths = - new ArrayList(nttp1.getPpAttributes().getUsedWavelength().values()); + new ArrayList<>(nttp1.getPpAttributes().getUsedWavelength().values()); if (usedWavelengths.isEmpty()) { used = false; } @@ -138,8 +138,8 @@ public class PceOpticalNode implements PceNode { case SRG : List srgAvailableWL = - new ArrayList(node1.getSrgAttributes().getAvailableWavelengths().values()); - if (srgAvailableWL == null) { + new ArrayList<>(node1.getSrgAttributes().nonnullAvailableWavelengths().values()); + if (srgAvailableWL.isEmpty()) { this.valid = false; LOG.error("initWLlist: SRG AvailableWavelengths is empty for node {}", this); return; @@ -153,8 +153,8 @@ public class PceOpticalNode implements PceNode { case DEGREE : List degAvailableWL = - new ArrayList(node1.getDegreeAttributes().getAvailableWavelengths().values()); - if (degAvailableWL == null) { + new ArrayList<>(node1.getDegreeAttributes().nonnullAvailableWavelengths().values()); + if (degAvailableWL.isEmpty()) { this.valid = false; LOG.error("initWLlist: DEG AvailableWavelengths is empty for node {}", this); return; @@ -191,8 +191,8 @@ public class PceOpticalNode implements PceNode { this.node.augmentation(org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang .ietf.network.topology.rev180226.Node1.class); List allTps = new ArrayList(nodeTp.getTerminationPoint().values()); - if (allTps == null) { + .node.TerminationPoint> allTps = new ArrayList<>(nodeTp.nonnullTerminationPoint().values()); + if (allTps.isEmpty()) { this.valid = false; LOG.error("initXndrTps: XPONDER TerminationPoint list is empty for node {}", this); return; 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 c431d8b6f..ec1043fc1 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 @@ -101,9 +101,9 @@ public class PceOtnNode implements PceNode { = this.node.augmentation(org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang .ietf.network.topology.rev180226.Node1.class); List allTps = new ArrayList(nodeTp.getTerminationPoint().values()); + .node.TerminationPoint> allTps = new ArrayList<>(nodeTp.nonnullTerminationPoint().values()); this.valid = false; - if (allTps == null) { + if (allTps.isEmpty()) { LOG.error("PceOtnNode: initXndrTps: XPONDER TerminationPoint list is empty for node {}", this); return; } @@ -188,8 +188,8 @@ public class PceOtnNode implements PceNode { for (TpId nwTp : netwTps) { for (TpId clTp : clientTps) { @Nullable - List nblList = new ArrayList(node.augmentation(Node1.class).getSwitchingPools() - .getOduSwitchingPools().values().stream().findFirst().get().getNonBlockingList().values()); + List nblList = new ArrayList<>(node.augmentation(Node1.class).getSwitchingPools() + .nonnullOduSwitchingPools().values().stream().findFirst().get().getNonBlockingList().values()); for (NonBlockingList nbl : nblList) { if (nbl.getTpList().contains(clTp) && nbl.getTpList().contains(nwTp)) { usableXpdrClientTps.add(clTp); @@ -207,8 +207,8 @@ public class PceOtnNode implements PceNode { if (clientTps == null && netwTps != null && nbClient == 0 && nbNetw == 2) { netwTps.sort(Comparator.comparing(TpId::getValue)); @Nullable - List nblList = new ArrayList(node.augmentation(Node1.class).getSwitchingPools() - .getOduSwitchingPools().values().stream().findFirst().get().getNonBlockingList().values()); + List nblList = new ArrayList<>(node.augmentation(Node1.class).getSwitchingPools() + .nonnullOduSwitchingPools().values().stream().findFirst().get().getNonBlockingList().values()); for (NonBlockingList nbl : nblList) { for (TpId nwTp : netwTps) { if (nbl.getTpList().contains(nwTp)) { @@ -306,9 +306,9 @@ public class PceOtnNode implements PceNode { node.augmentation( org.opendaylight.yang.gen.v1.http.org.openroadm.otn.network.topology.rev181130.Node1.class); SwitchingPools sp = node1.getSwitchingPools(); - List osp = new ArrayList(sp.getOduSwitchingPools().values()); + List osp = new ArrayList<>(sp.nonnullOduSwitchingPools().values()); for (OduSwitchingPools ospx : osp) { - List nbl = new ArrayList(ospx.getNonBlockingList().values()); + List nbl = new ArrayList<>(ospx.nonnullNonBlockingList().values()); for (NonBlockingList nbll : nbl) { if (nbll.getAvailableInterconnectBandwidth().toJava() >= neededBW && nbll.getTpList() != null && nbll.getTpList().contains(tp1.getTpId()) && nbll.getTpList().contains(tp2.getTpId())) { diff --git a/pce/src/main/java/org/opendaylight/transportpce/pce/service/PathComputationServiceImpl.java b/pce/src/main/java/org/opendaylight/transportpce/pce/service/PathComputationServiceImpl.java index c9fd310df..bd706b278 100644 --- a/pce/src/main/java/org/opendaylight/transportpce/pce/service/PathComputationServiceImpl.java +++ b/pce/src/main/java/org/opendaylight/transportpce/pce/service/PathComputationServiceImpl.java @@ -15,6 +15,7 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.Executors; +import java.util.stream.Collectors; import org.opendaylight.mdsal.binding.api.NotificationPublishService; import org.opendaylight.mdsal.binding.dom.codec.spi.BindingDOMCodecServices; import org.opendaylight.transportpce.common.network.NetworkTransactionService; @@ -177,7 +178,8 @@ public class PathComputationServiceImpl implements PathComputationService { GnpyResponse respZtoA = generateGnpyResponse(gnpyZtoA.getResponse(),"Z-to-A"); listResponse.add(respZtoA); } - output.setGnpyResponse(listResponse); + output.setGnpyResponse(listResponse.stream() + .collect(Collectors.toMap(GnpyResponse::key, gnpyResponse -> gnpyResponse))); if (Boolean.FALSE.equals(sendingPCE.getSuccess()) || (path == null)) { configurationResponseCommon.setAckFinalIndicator("Yes") @@ -259,7 +261,10 @@ public class PathComputationServiceImpl implements PathComputationService { .setAccumulativeValue(pathMetricGnpy.getAccumulativeValue()).build(); gnpyPathMetricList.add(pathMetric); } - PathProperties pathProperties = new PathPropertiesBuilder().setPathMetric(gnpyPathMetricList).build(); + PathProperties pathProperties = new PathPropertiesBuilder() + .setPathMetric(gnpyPathMetricList.stream() + .collect(Collectors.toMap(PathMetric::key, pathMetric -> pathMetric))) + .build(); PathCase gnpyPathCase = new PathCaseBuilder().setPathProperties(pathProperties).build(); respType = gnpyPathCase; feasible = true; diff --git a/pce/src/test/java/org/opendaylight/transportpce/pce/impl/PceProviderTest.java b/pce/src/test/java/org/opendaylight/transportpce/pce/impl/PceProviderTest.java index 506fad9c6..1a69d3db4 100644 --- a/pce/src/test/java/org/opendaylight/transportpce/pce/impl/PceProviderTest.java +++ b/pce/src/test/java/org/opendaylight/transportpce/pce/impl/PceProviderTest.java @@ -8,7 +8,6 @@ package org.opendaylight.transportpce.pce.impl; -import static org.mockito.ArgumentMatchers.anyObject; import static org.mockito.ArgumentMatchers.eq; import org.eclipse.jdt.annotation.NonNull; @@ -61,11 +60,9 @@ public class PceProviderTest extends AbstractTest { } }; - - Mockito .when(rpcService - .registerRpcImplementation(eq(TransportpcePceService.class), anyObject())) + .registerRpcImplementation(eq(TransportpcePceService.class), Mockito.any())) .thenReturn(rpcRegistration); pceProvider.init(); pceProvider.close(); diff --git a/tests/transportpce_tests/1.2.1/test_gnpy.py b/tests/transportpce_tests/1.2.1/test_gnpy.py index 80a92bf0d..09812bd8d 100644 --- a/tests/transportpce_tests/1.2.1/test_gnpy.py +++ b/tests/transportpce_tests/1.2.1/test_gnpy.py @@ -102,12 +102,12 @@ class TransportGNPYtesting(unittest.TestCase): self.assertEqual(res['output']['configuration-response-common'][ 'response-message'], 'Path is calculated by PCE') - self.assertEqual(res['output']['gnpy-response'][0]['path-dir'], - 'A-to-Z') - self.assertEqual(res['output']['gnpy-response'][0]['feasibility'], True) self.assertEqual(res['output']['gnpy-response'][1]['path-dir'], - 'Z-to-A') + 'A-to-Z') self.assertEqual(res['output']['gnpy-response'][1]['feasibility'], True) + self.assertEqual(res['output']['gnpy-response'][0]['path-dir'], + 'Z-to-A') + self.assertEqual(res['output']['gnpy-response'][0]['feasibility'], True) time.sleep(5) # Path computed by PCE is not feasible by GNPy and GNPy cannot find @@ -130,13 +130,13 @@ class TransportGNPYtesting(unittest.TestCase): self.assertEqual(res['output']['configuration-response-common'][ 'response-message'], 'No path available by PCE and GNPy ') - self.assertEqual(res['output']['gnpy-response'][0]['path-dir'], + self.assertEqual(res['output']['gnpy-response'][1]['path-dir'], 'A-to-Z') - self.assertEqual(res['output']['gnpy-response'][0]['feasibility'], + self.assertEqual(res['output']['gnpy-response'][1]['feasibility'], False) - self.assertEqual(res['output']['gnpy-response'][1]['path-dir'], + self.assertEqual(res['output']['gnpy-response'][0]['path-dir'], 'Z-to-A') - self.assertEqual(res['output']['gnpy-response'][1]['feasibility'], + self.assertEqual(res['output']['gnpy-response'][0]['feasibility'], False) time.sleep(5) @@ -158,12 +158,12 @@ class TransportGNPYtesting(unittest.TestCase): self.assertEqual(res['output']['configuration-response-common'][ 'response-message'], 'Path is calculated by GNPy') - self.assertEqual(res['output']['gnpy-response'][0]['path-dir'], - 'A-to-Z') - self.assertEqual(res['output']['gnpy-response'][0]['feasibility'], True) self.assertEqual(res['output']['gnpy-response'][1]['path-dir'], - 'Z-to-A') + 'A-to-Z') self.assertEqual(res['output']['gnpy-response'][1]['feasibility'], True) + self.assertEqual(res['output']['gnpy-response'][0]['path-dir'], + 'Z-to-A') + self.assertEqual(res['output']['gnpy-response'][0]['feasibility'], True) time.sleep(5) # Not found path by PCE and GNPy cannot find another one -- 2.36.6