From bf2bf3ea3e0b45f2ae7df27bf4634214add7c0ce Mon Sep 17 00:00:00 2001 From: Yevgeny Khodorkovsky Date: Wed, 7 Aug 2013 21:31:48 -0700 Subject: [PATCH] Use Set for Subnet's NodeConnectors - Use Set instead of List in Subnet's aggreagated NodeConnectors to avoid handling dpulicates - Style fixes Change-Id: I037764503b5bc01791716a9f515825a706451153 Signed-off-by: Yevgeny Khodorkovsky --- .../northbound/SubnetsNorthboundJAXRS.java | 6 ++-- .../controller/switchmanager/Subnet.java | 8 ++--- .../switchmanager/SubnetConfig.java | 8 ++--- .../switchmanager/internal/SwitchManager.java | 36 ++++++++++--------- .../internal/SwitchManagerTest.java | 3 +- .../controller/devices/web/Devices.java | 3 +- 6 files changed, 30 insertions(+), 34 deletions(-) diff --git a/opendaylight/northbound/subnets/src/main/java/org/opendaylight/controller/subnets/northbound/SubnetsNorthboundJAXRS.java b/opendaylight/northbound/subnets/src/main/java/org/opendaylight/controller/subnets/northbound/SubnetsNorthboundJAXRS.java index e3c2189bf9..17780fc70d 100644 --- a/opendaylight/northbound/subnets/src/main/java/org/opendaylight/controller/subnets/northbound/SubnetsNorthboundJAXRS.java +++ b/opendaylight/northbound/subnets/src/main/java/org/opendaylight/controller/subnets/northbound/SubnetsNorthboundJAXRS.java @@ -7,9 +7,7 @@ */ package org.opendaylight.controller.subnets.northbound; -import java.util.ArrayList; import java.util.HashSet; -import java.util.List; import java.util.Set; import javax.ws.rs.Consumes; @@ -159,7 +157,7 @@ public class SubnetsNorthboundJAXRS { throw new ResourceNotFoundException(RestMessages.NOCONTAINER.toString()); } - SubnetConfig cfgObject = new SubnetConfig(subnetName, subnet, new ArrayList(0)); + SubnetConfig cfgObject = new SubnetConfig(subnetName, subnet, new HashSet(0)); Status status = switchManager.addSubnet(cfgObject); if (status.isSuccess()) { return Response.status(Response.Status.CREATED).build(); @@ -387,7 +385,7 @@ public class SubnetsNorthboundJAXRS { } else { Status st; boolean successful = true; - List ports = subnetConf.getNodePorts(); + Set ports = subnetConf.getNodePorts(); if (action.equals("add")) { // add new ports diff --git a/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/Subnet.java b/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/Subnet.java index 6a8eea2d0c..55a7ecb9cd 100644 --- a/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/Subnet.java +++ b/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/Subnet.java @@ -56,12 +56,8 @@ public class Subnet implements Cloneable, Serializable { * @param sp Set of NodeConnectors to add to the subnet */ public void addNodeConnectors(Set sp) { - if (sp == null) { - return; - } - - for (NodeConnector p : sp) { - this.nodeConnectors.add(p); + if (sp != null) { + this.nodeConnectors.addAll(sp); } } diff --git a/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/SubnetConfig.java b/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/SubnetConfig.java index 52b5f5255d..4ed9934b27 100644 --- a/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/SubnetConfig.java +++ b/opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/SubnetConfig.java @@ -48,13 +48,13 @@ public class SubnetConfig implements Cloneable, Serializable { private String subnet; // A.B.C.D/MM Where A.B.C.D is the Default // Gateway IP (L3) or ARP Querier IP (L2 @XmlElement - private List nodePorts; // datapath ID/port list: + private Set nodePorts; // datapath ID/port list: // xx:xx:xx:xx:xx:xx:xx:xx/a,b,c-m,r-t,y public SubnetConfig() { } - public SubnetConfig(String desc, String sub, List sp) { + public SubnetConfig(String desc, String sub, Set sp) { name = desc; subnet = sub; nodePorts = sp; @@ -63,14 +63,14 @@ public class SubnetConfig implements Cloneable, Serializable { public SubnetConfig(SubnetConfig subnetConfig) { name = subnetConfig.name; subnet = subnetConfig.subnet; - nodePorts = new ArrayList(subnetConfig.nodePorts); + nodePorts = new HashSet(subnetConfig.nodePorts); } public String getName() { return name; } - public List getNodePorts() { + public Set getNodePorts() { return nodePorts; } diff --git a/opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManager.java b/opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManager.java index f50f303a3f..4c4adf0478 100644 --- a/opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManager.java +++ b/opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManager.java @@ -92,7 +92,7 @@ CommandProvider { private static final String SAVE = "Save"; private String subnetFileName, spanFileName, switchConfigFileName; private final List spanNodeConnectors = new CopyOnWriteArrayList(); - // set of Subnets keyed by the InetAddress + // Collection of Subnets keyed by the InetAddress private ConcurrentMap subnets; private ConcurrentMap subnetsConfigList; private ConcurrentMap spanConfigList; @@ -401,19 +401,21 @@ CommandProvider { Set sp = conf.getSubnetNodeConnectors(); subnet.addNodeConnectors(sp); } - boolean result = false; + boolean putNewSubnet = false; if(subnetCurr == null) { if(subnets.putIfAbsent(conf.getIPnum(), subnet) == null) { - result = true; + putNewSubnet = true; } } else { - result = subnets.replace(conf.getIPnum(), subnetCurr, subnet); + putNewSubnet = subnets.replace(conf.getIPnum(), subnetCurr, subnet); } - if(!result) { + if(!putNewSubnet) { String msg = "Cluster conflict: Conflict while adding the subnet " + conf.getIPnum(); return new Status(StatusCode.CONFLICT, msg); } - } else { // This is the deletion of the whole subnet + + // Subnet removal case + } else { subnets.remove(conf.getIPnum()); } return new Status(StatusCode.SUCCESS); @@ -435,7 +437,7 @@ CommandProvider { return new Status(StatusCode.SUCCESS); } - private Status addRemoveSubnet(SubnetConfig conf, boolean add) { + private Status addRemoveSubnet(SubnetConfig conf, boolean isAdding) { // Valid config check if (!conf.isValidConfig()) { String msg = "Invalid Subnet configuration"; @@ -443,13 +445,13 @@ CommandProvider { return new Status(StatusCode.BADREQUEST, msg); } - if (add) { + if (isAdding) { // Presence check if (subnetsConfigList.containsKey(conf.getName())) { return new Status(StatusCode.CONFLICT, - "Same subnet config already exists"); + "Subnet with the specified name already configured."); } - // Semantyc check + // Semantic check Status rc = semanticCheck(conf); if (!rc.isSuccess()) { return rc; @@ -457,13 +459,13 @@ CommandProvider { } // Update Database - Status rc = updateDatabase(conf, add); + Status rc = updateDatabase(conf, isAdding); if (rc.isSuccess()) { // Update Configuration - rc = updateConfig(conf, add); + rc = updateConfig(conf, isAdding); if(!rc.isSuccess()) { - updateDatabase(conf, (!add)); + updateDatabase(conf, (!isAdding)); } } @@ -512,8 +514,8 @@ CommandProvider { Subnet sub = subCurr.clone(); Set sp = confCurr.getNodeConnectors(switchPorts); sub.addNodeConnectors(sp); - boolean subnetsReplace = subnets.replace(confCurr.getIPnum(), subCurr, sub); - if (!subnetsReplace) { + boolean subnetsReplaced = subnets.replace(confCurr.getIPnum(), subCurr, sub); + if (!subnetsReplaced) { String msg = "Cluster conflict: Conflict while adding ports to the subnet " + name; return new Status(StatusCode.CONFLICT, msg); } @@ -521,8 +523,8 @@ CommandProvider { // Update Configuration SubnetConfig conf = confCurr.clone(); conf.addNodeConnectors(switchPorts); - boolean result = subnetsConfigList.replace(name, confCurr, conf); - if (!result) { + boolean configReplaced = subnetsConfigList.replace(name, confCurr, conf); + if (!configReplaced) { // TODO: recovery using Transactionality String msg = "Cluster conflict: Conflict while adding ports to the subnet " + name; return new Status(StatusCode.CONFLICT, msg); diff --git a/opendaylight/switchmanager/implementation/src/test/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerTest.java b/opendaylight/switchmanager/implementation/src/test/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerTest.java index a233ad4b3e..d74c3d3630 100644 --- a/opendaylight/switchmanager/implementation/src/test/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerTest.java +++ b/opendaylight/switchmanager/implementation/src/test/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerTest.java @@ -8,7 +8,6 @@ package org.opendaylight.controller.switchmanager.internal; -import java.util.ArrayList; import java.util.HashSet; import java.util.Set; @@ -33,7 +32,7 @@ public class SwitchManagerTest { SwitchManager switchmgr = new SwitchManager(); switchmgr.startUp(); - ArrayList portList = new ArrayList(); + Set portList = new HashSet(); portList.add("1/1"); portList.add("1/2"); portList.add("1/3"); diff --git a/opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/Devices.java b/opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/Devices.java index ca1dd97137..37798ac137 100644 --- a/opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/Devices.java +++ b/opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/Devices.java @@ -10,6 +10,7 @@ package org.opendaylight.controller.devices.web; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -438,7 +439,7 @@ public class Devices implements IDaylightWeb { ISwitchManager switchManager = (ISwitchManager) ServiceHelper .getInstance(ISwitchManager.class, containerName, this); SubnetConfig cfgObject = new SubnetConfig(gatewayName, - gatewayIPAddress, new ArrayList()); + gatewayIPAddress, new HashSet()); Status result = switchManager.addSubnet(cfgObject); if (result.isSuccess()) { resultBean.setStatus(true); -- 2.36.6