Committed by
Gerrit Code Review
ONOS-4665 OSPF refactoring - review comments
Change-Id: I6846e414f441e3df53e2e88873158ac579471f26
Showing
2 changed files
with
106 additions
and
73 deletions
| ... | @@ -60,7 +60,7 @@ public final class OspfConfigUtil { | ... | @@ -60,7 +60,7 @@ public final class OspfConfigUtil { |
| 60 | /** | 60 | /** |
| 61 | * Returns list of OSPF process from the json nodes. | 61 | * Returns list of OSPF process from the json nodes. |
| 62 | * | 62 | * |
| 63 | - * @param jsonNodes json node instance | 63 | + * @param jsonNodes represents one or more OSPF process configuration |
| 64 | * @return list of OSPF processes. | 64 | * @return list of OSPF processes. |
| 65 | */ | 65 | */ |
| 66 | public static List<OspfProcess> processes(JsonNode jsonNodes) { | 66 | public static List<OspfProcess> processes(JsonNode jsonNodes) { |
| ... | @@ -68,82 +68,24 @@ public final class OspfConfigUtil { | ... | @@ -68,82 +68,24 @@ public final class OspfConfigUtil { |
| 68 | if (jsonNodes == null) { | 68 | if (jsonNodes == null) { |
| 69 | return ospfProcesses; | 69 | return ospfProcesses; |
| 70 | } | 70 | } |
| 71 | + //From each Process nodes, get area and related interface details. | ||
| 71 | jsonNodes.forEach(jsonNode -> { | 72 | jsonNodes.forEach(jsonNode -> { |
| 72 | List<OspfArea> areas = new ArrayList<>(); | 73 | List<OspfArea> areas = new ArrayList<>(); |
| 74 | + //Get configured areas for the process. | ||
| 73 | for (JsonNode areaNode : jsonNode.path(AREAS)) { | 75 | for (JsonNode areaNode : jsonNode.path(AREAS)) { |
| 74 | List<OspfInterface> interfaceList = new ArrayList<>(); | 76 | List<OspfInterface> interfaceList = new ArrayList<>(); |
| 75 | for (JsonNode interfaceNode : areaNode.path(INTERFACE)) { | 77 | for (JsonNode interfaceNode : areaNode.path(INTERFACE)) { |
| 76 | - OspfInterface ospfInterface = new OspfInterfaceImpl(); | 78 | + OspfInterface ospfInterface = interfaceDetails(interfaceNode); |
| 77 | - String index = interfaceNode.path(INTERFACEINDEX).asText(); | 79 | + if (ospfInterface != null) { |
| 78 | - if (isValidDigit(index)) { | 80 | + interfaceList.add(ospfInterface); |
| 79 | - ospfInterface.setInterfaceIndex(Integer.parseInt(index)); | ||
| 80 | - } else { | ||
| 81 | - log.debug("Wrong interface index: {}", index); | ||
| 82 | - continue; | ||
| 83 | } | 81 | } |
| 84 | - ospfInterface.setIpAddress(getInterfaceIp(ospfInterface.interfaceIndex())); | ||
| 85 | - ospfInterface.setIpNetworkMask(Ip4Address.valueOf(getInterfaceMask( | ||
| 86 | - ospfInterface.interfaceIndex()))); | ||
| 87 | - ospfInterface.setBdr(OspfUtil.DEFAULTIP); | ||
| 88 | - ospfInterface.setDr(OspfUtil.DEFAULTIP); | ||
| 89 | - String helloInterval = interfaceNode.path(HELLOINTERVAL).asText(); | ||
| 90 | - if (isValidDigit(helloInterval)) { | ||
| 91 | - ospfInterface.setHelloIntervalTime(Integer.parseInt(helloInterval)); | ||
| 92 | - } else { | ||
| 93 | - log.debug("Wrong hello interval: {}", helloInterval); | ||
| 94 | - continue; | ||
| 95 | - } | ||
| 96 | - String routerDeadInterval = interfaceNode.path(ROUTERDEADINTERVAL).asText(); | ||
| 97 | - if (isValidDigit(routerDeadInterval)) { | ||
| 98 | - ospfInterface.setRouterDeadIntervalTime(Integer.parseInt(routerDeadInterval)); | ||
| 99 | - } else { | ||
| 100 | - log.debug("Wrong routerDeadInterval: {}", routerDeadInterval); | ||
| 101 | - continue; | ||
| 102 | - } | ||
| 103 | - String interfaceType = interfaceNode.path(INTERFACETYPE).asText(); | ||
| 104 | - if (isValidDigit(interfaceType)) { | ||
| 105 | - ospfInterface.setInterfaceType(Integer.parseInt(interfaceType)); | ||
| 106 | - } else { | ||
| 107 | - log.debug("Wrong interfaceType: {}", interfaceType); | ||
| 108 | - continue; | ||
| 109 | - } | ||
| 110 | - ospfInterface.setReTransmitInterval(OspfUtil.RETRANSMITINTERVAL); | ||
| 111 | - ospfInterface.setMtu(OspfUtil.MTU); | ||
| 112 | - ospfInterface.setRouterPriority(OspfUtil.ROUTER_PRIORITY); | ||
| 113 | - interfaceList.add(ospfInterface); | ||
| 114 | - } | ||
| 115 | - OspfArea area = new OspfAreaImpl(); | ||
| 116 | - String areaId = areaNode.path(AREAID).asText(); | ||
| 117 | - if (isValidIpAddress(areaId)) { | ||
| 118 | - area.setAreaId(Ip4Address.valueOf(areaId)); | ||
| 119 | - } else { | ||
| 120 | - log.debug("Wrong areaId: {}", areaId); | ||
| 121 | - continue; | ||
| 122 | - } | ||
| 123 | - String routerId = areaNode.path(ROUTERID).asText(); | ||
| 124 | - if (isValidIpAddress(routerId)) { | ||
| 125 | - area.setRouterId(Ip4Address.valueOf(routerId)); | ||
| 126 | - } else { | ||
| 127 | - log.debug("Wrong routerId: {}", routerId); | ||
| 128 | - continue; | ||
| 129 | } | 82 | } |
| 130 | - String routingCapability = areaNode.path(EXTERNALROUTINGCAPABILITY).asText(); | 83 | + //Get the area details |
| 131 | - if (isBoolean(routingCapability)) { | 84 | + OspfArea area = areaDetails(areaNode); |
| 132 | - area.setExternalRoutingCapability(Boolean.valueOf(routingCapability)); | 85 | + if (area != null) { |
| 133 | - } else { | 86 | + area.setOspfInterfaceList(interfaceList); |
| 134 | - log.debug("Wrong routingCapability: {}", routingCapability); | 87 | + areas.add(area); |
| 135 | - continue; | ||
| 136 | } | 88 | } |
| 137 | - String isOpaqueEnabled = areaNode.path(ISOPAQUE).asText(); | ||
| 138 | - if (isBoolean(isOpaqueEnabled)) { | ||
| 139 | - area.setIsOpaqueEnabled(Boolean.valueOf(isOpaqueEnabled)); | ||
| 140 | - } else { | ||
| 141 | - log.debug("Wrong isOpaqueEnabled: {}", isOpaqueEnabled); | ||
| 142 | - continue; | ||
| 143 | - } | ||
| 144 | - area.setOptions(OspfUtil.HELLO_PACKET_OPTIONS); | ||
| 145 | - area.setOspfInterfaceList(interfaceList); | ||
| 146 | - areas.add(area); | ||
| 147 | } | 89 | } |
| 148 | OspfProcess process = new OspfProcessImpl(); | 90 | OspfProcess process = new OspfProcessImpl(); |
| 149 | process.setProcessId(jsonNode.path(PROCESSID).asText()); | 91 | process.setProcessId(jsonNode.path(PROCESSID).asText()); |
| ... | @@ -294,4 +236,97 @@ public final class OspfConfigUtil { | ... | @@ -294,4 +236,97 @@ public final class OspfConfigUtil { |
| 294 | 236 | ||
| 295 | return status; | 237 | return status; |
| 296 | } | 238 | } |
| 297 | -} | ||
| ... | \ No newline at end of file | ... | \ No newline at end of file |
| 239 | + | ||
| 240 | + /** | ||
| 241 | + * Returns OSPF area instance from configuration. | ||
| 242 | + * | ||
| 243 | + * @param areaNode area configuration | ||
| 244 | + * @return OSPF area instance | ||
| 245 | + */ | ||
| 246 | + private static OspfArea areaDetails(JsonNode areaNode) { | ||
| 247 | + OspfArea area = new OspfAreaImpl(); | ||
| 248 | + String areaId = areaNode.path(AREAID).asText(); | ||
| 249 | + if (isValidIpAddress(areaId)) { | ||
| 250 | + area.setAreaId(Ip4Address.valueOf(areaId)); | ||
| 251 | + } else { | ||
| 252 | + log.debug("Wrong areaId: {}", areaId); | ||
| 253 | + return null; | ||
| 254 | + } | ||
| 255 | + String routerId = areaNode.path(ROUTERID).asText(); | ||
| 256 | + if (isValidIpAddress(routerId)) { | ||
| 257 | + area.setRouterId(Ip4Address.valueOf(routerId)); | ||
| 258 | + } else { | ||
| 259 | + log.debug("Wrong routerId: {}", routerId); | ||
| 260 | + return null; | ||
| 261 | + } | ||
| 262 | + String routingCapability = areaNode.path(EXTERNALROUTINGCAPABILITY).asText(); | ||
| 263 | + if (isBoolean(routingCapability)) { | ||
| 264 | + area.setExternalRoutingCapability(Boolean.valueOf(routingCapability)); | ||
| 265 | + } else { | ||
| 266 | + log.debug("Wrong routingCapability: {}", routingCapability); | ||
| 267 | + return null; | ||
| 268 | + } | ||
| 269 | + String isOpaqueEnabled = areaNode.path(ISOPAQUE).asText(); | ||
| 270 | + if (isBoolean(isOpaqueEnabled)) { | ||
| 271 | + area.setIsOpaqueEnabled(Boolean.valueOf(isOpaqueEnabled)); | ||
| 272 | + } else { | ||
| 273 | + log.debug("Wrong isOpaqueEnabled: {}", isOpaqueEnabled); | ||
| 274 | + return null; | ||
| 275 | + } | ||
| 276 | + area.setOptions(OspfUtil.HELLO_PACKET_OPTIONS); | ||
| 277 | + | ||
| 278 | + return area; | ||
| 279 | + } | ||
| 280 | + | ||
| 281 | + /** | ||
| 282 | + * Returns OSPF interface instance from configuration. | ||
| 283 | + * | ||
| 284 | + * @param interfaceNode interface configuration | ||
| 285 | + * @return OSPF interface instance | ||
| 286 | + */ | ||
| 287 | + private static OspfInterface interfaceDetails(JsonNode interfaceNode) { | ||
| 288 | + OspfInterface ospfInterface = new OspfInterfaceImpl(); | ||
| 289 | + String index = interfaceNode.path(INTERFACEINDEX).asText(); | ||
| 290 | + if (isValidDigit(index)) { | ||
| 291 | + ospfInterface.setInterfaceIndex(Integer.parseInt(index)); | ||
| 292 | + } else { | ||
| 293 | + log.debug("Wrong interface index: {}", index); | ||
| 294 | + return null; | ||
| 295 | + } | ||
| 296 | + Ip4Address interfaceIp = getInterfaceIp(ospfInterface.interfaceIndex()); | ||
| 297 | + if (interfaceIp.equals(OspfUtil.DEFAULTIP)) { | ||
| 298 | + return null; | ||
| 299 | + } | ||
| 300 | + ospfInterface.setIpAddress(interfaceIp); | ||
| 301 | + ospfInterface.setIpNetworkMask(Ip4Address.valueOf(getInterfaceMask( | ||
| 302 | + ospfInterface.interfaceIndex()))); | ||
| 303 | + ospfInterface.setBdr(OspfUtil.DEFAULTIP); | ||
| 304 | + ospfInterface.setDr(OspfUtil.DEFAULTIP); | ||
| 305 | + String helloInterval = interfaceNode.path(HELLOINTERVAL).asText(); | ||
| 306 | + if (isValidDigit(helloInterval)) { | ||
| 307 | + ospfInterface.setHelloIntervalTime(Integer.parseInt(helloInterval)); | ||
| 308 | + } else { | ||
| 309 | + log.debug("Wrong hello interval: {}", helloInterval); | ||
| 310 | + return null; | ||
| 311 | + } | ||
| 312 | + String routerDeadInterval = interfaceNode.path(ROUTERDEADINTERVAL).asText(); | ||
| 313 | + if (isValidDigit(routerDeadInterval)) { | ||
| 314 | + ospfInterface.setRouterDeadIntervalTime(Integer.parseInt(routerDeadInterval)); | ||
| 315 | + } else { | ||
| 316 | + log.debug("Wrong routerDeadInterval: {}", routerDeadInterval); | ||
| 317 | + return null; | ||
| 318 | + } | ||
| 319 | + String interfaceType = interfaceNode.path(INTERFACETYPE).asText(); | ||
| 320 | + if (isValidDigit(interfaceType)) { | ||
| 321 | + ospfInterface.setInterfaceType(Integer.parseInt(interfaceType)); | ||
| 322 | + } else { | ||
| 323 | + log.debug("Wrong interfaceType: {}", interfaceType); | ||
| 324 | + return null; | ||
| 325 | + } | ||
| 326 | + ospfInterface.setReTransmitInterval(OspfUtil.RETRANSMITINTERVAL); | ||
| 327 | + ospfInterface.setMtu(OspfUtil.MTU); | ||
| 328 | + ospfInterface.setRouterPriority(OspfUtil.ROUTER_PRIORITY); | ||
| 329 | + | ||
| 330 | + return ospfInterface; | ||
| 331 | + } | ||
| 332 | +} | ... | ... |
| ... | @@ -19,7 +19,6 @@ import com.fasterxml.jackson.databind.JsonNode; | ... | @@ -19,7 +19,6 @@ import com.fasterxml.jackson.databind.JsonNode; |
| 19 | import com.fasterxml.jackson.databind.ObjectMapper; | 19 | import com.fasterxml.jackson.databind.ObjectMapper; |
| 20 | import org.junit.After; | 20 | import org.junit.After; |
| 21 | import org.junit.Before; | 21 | import org.junit.Before; |
| 22 | -import org.junit.Ignore; | ||
| 23 | import org.junit.Test; | 22 | import org.junit.Test; |
| 24 | import org.onosproject.ospf.controller.OspfProcess; | 23 | import org.onosproject.ospf.controller.OspfProcess; |
| 25 | 24 | ||
| ... | @@ -33,7 +32,6 @@ import static org.hamcrest.MatcherAssert.assertThat; | ... | @@ -33,7 +32,6 @@ import static org.hamcrest.MatcherAssert.assertThat; |
| 33 | /** | 32 | /** |
| 34 | * Unit test class for OspfJsonParsingUtilTest. | 33 | * Unit test class for OspfJsonParsingUtilTest. |
| 35 | */ | 34 | */ |
| 36 | -@Ignore | ||
| 37 | public class OspfConfigUtilTest { | 35 | public class OspfConfigUtilTest { |
| 38 | private ObjectMapper mapper; | 36 | private ObjectMapper mapper; |
| 39 | private JsonNode jsonNode; | 37 | private JsonNode jsonNode; |
| ... | @@ -81,4 +79,4 @@ public class OspfConfigUtilTest { | ... | @@ -81,4 +79,4 @@ public class OspfConfigUtilTest { |
| 81 | ospfProcessList = OspfConfigUtil.processes(jsonNode); | 79 | ospfProcessList = OspfConfigUtil.processes(jsonNode); |
| 82 | assertThat(ospfProcessList, is(notNullValue())); | 80 | assertThat(ospfProcessList, is(notNullValue())); |
| 83 | } | 81 | } |
| 84 | -} | ||
| ... | \ No newline at end of file | ... | \ No newline at end of file |
| 82 | +} | ... | ... |
-
Please register or login to post a comment