From: Alessandro Boch Date: Mon, 21 Oct 2013 23:46:15 +0000 (-0700) Subject: Return singleton empty collection instead of null in Read service code X-Git-Tag: jenkins-controller-bulk-release-prepare-only-2-1~567 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=18e1184615fd939644d3660e5edcfbb676c187fa Return singleton empty collection instead of null in Read service code Change-Id: I763bdd6178047e3de43158176c29a8bd6f478b25 Signed-off-by: Alessandro Boch --- diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/FlowStatisticsConverter.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/FlowStatisticsConverter.java index 6873b528a2..74af3db5ef 100644 --- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/FlowStatisticsConverter.java +++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/FlowStatisticsConverter.java @@ -9,6 +9,7 @@ package org.opendaylight.controller.protocol_plugin.openflow.internal; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.opendaylight.controller.protocol_plugin.openflow.vendorextension.v6extension.V6StatsReply; @@ -34,11 +35,10 @@ public class FlowStatisticsConverter { private List flowOnNodeList; public FlowStatisticsConverter(List statsList) { - if (statsList == null) {// || statsList.isEmpty()) { - this.ofStatsList = new ArrayList(1); // dummy list + if (statsList == null) { + this.ofStatsList = Collections.emptyList(); } else { - this.ofStatsList = statsList; // new - // ArrayList(statsList); + this.ofStatsList = statsList; } this.flowOnNodeList = null; } diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/OFStatisticsManager.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/OFStatisticsManager.java index 48b46eac85..8a86d2f256 100644 --- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/OFStatisticsManager.java +++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/OFStatisticsManager.java @@ -86,7 +86,6 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim private ConcurrentMap> descStatistics; private ConcurrentMap> portStatistics; private ConcurrentMap> tableStatistics; - private List dummyList; private ConcurrentMap statisticsTimerTicks; protected BlockingQueue pendingStatsRequests; protected BlockingQueue switchPortStatsUpdated; @@ -191,7 +190,6 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim descStatistics = new ConcurrentHashMap>(); portStatistics = new ConcurrentHashMap>(); tableStatistics = new ConcurrentHashMap>(); - dummyList = new ArrayList(1); pendingStatsRequests = new LinkedBlockingQueue(getStatsQueueSize()); statisticsTimerTicks = new ConcurrentHashMap(INITIAL_SIZE); switchPortStatsUpdated = new LinkedBlockingQueue(INITIAL_SIZE); @@ -522,7 +520,7 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim List values = this.fetchStatisticsFromSwitch(switchId, statType, null); // If got a valid response update local cache and notify listeners - if (values != null && !values.isEmpty()) { + if (!values.isEmpty()) { switch (statType) { case FLOW: case VENDOR: @@ -589,7 +587,7 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim @SuppressWarnings("unchecked") private List fetchStatisticsFromSwitch(Long switchId, OFStatisticsType statsType, Object target) { - List values = null; + List values = Collections.emptyList(); String type = null; ISwitch sw = controller.getSwitch(switchId); @@ -608,7 +606,7 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim // Malformed request log.warn("Invalid target type for Flow stats request: {}", target.getClass()); - return null; + return Collections.emptyList(); } else { // Specific flow request match = (OFMatch) target; @@ -649,7 +647,7 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim // Malformed request log.warn("Invalid target type for Port stats request: {}", target.getClass()); - return null; + return Collections.emptyList(); } else { // Specific port request targetPort = (Short) target; @@ -676,7 +674,7 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim // Malformed request log.warn("Invalid table id for table stats request: {}", target.getClass()); - return null; + return Collections.emptyList(); } byte targetTable = (Byte) target; OFTableStatistics specificReq = new OFTableStatistics(); @@ -719,7 +717,7 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim * Check on emptiness as interference between add and get is still * possible on the inner list (the concurrentMap entry's value) */ - return (list == null || list.isEmpty()) ? this.dummyList + return (list == null || list.isEmpty()) ? Collections.emptyList() : (list.get(0) instanceof OFVendorStatistics) ? this .v6StatsListToOFStatsList(list) : list; } @@ -733,7 +731,7 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim * possible on the inner list (the concurrentMap entry's value) */ if (statsList == null || statsList.isEmpty()) { - return this.dummyList; + return Collections.emptyList(); } if (statsList.get(0) instanceof OFVendorStatistics) { @@ -766,22 +764,22 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim } } } - return this.dummyList; + return Collections.emptyList(); } /* * Converts the v6 vendor statistics to the OFStatistics */ - private List v6StatsListToOFStatsList( - List statistics) { + private List v6StatsListToOFStatsList(List statistics) { + if (statistics == null || statistics.isEmpty()) { + return Collections.emptyList(); + } List v6statistics = new ArrayList(); - if (statistics != null && !statistics.isEmpty()) { - for (OFStatistics stats : statistics) { - if (stats instanceof OFVendorStatistics) { - List r = getV6ReplyStatistics((OFVendorStatistics) stats); - if (r != null) { - v6statistics.addAll(r); - } + for (OFStatistics stats : statistics) { + if (stats instanceof OFVendorStatistics) { + List r = getV6ReplyStatistics((OFVendorStatistics) stats); + if (r != null) { + v6statistics.addAll(r); } } } @@ -792,8 +790,10 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim OFVendorStatistics stat) { int length = stat.getLength(); List results = new ArrayList(); - if (length < 12) - return null; // Nicira Hdr is 12 bytes. We need atleast that much + if (length < 12) { + // Nicira Hdr is 12 bytes. We need at least that much + return Collections.emptyList(); + } ByteBuffer data = ByteBuffer.allocate(length); stat.writeTo(data); data.rewind(); @@ -805,7 +805,7 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim int vendor = data.getInt(); // first 4 bytes is vendor id. if (vendor != V6StatsRequest.NICIRA_VENDOR_ID) { log.warn("Unexpected vendor id: 0x{}", Integer.toHexString(vendor)); - return null; + return Collections.emptyList(); } else { // go ahead by 8 bytes which is 8 bytes of 0 data.getLong(); // should be all 0's @@ -818,12 +818,14 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim while (length > 0) { v6statsreply = new V6StatsReply(); min_len = v6statsreply.getLength(); - if (length < v6statsreply.getLength()) + if (length < v6statsreply.getLength()) { break; + } v6statsreply.setActionFactory(stat.getActionFactory()); v6statsreply.readFrom(data); - if (v6statsreply.getLength() < min_len) + if (v6statsreply.getLength() < min_len) { break; + } v6statsreply.setVendorId(vendor); log.trace("V6StatsReply: {}", v6statsreply); length -= v6statsreply.getLength(); @@ -845,17 +847,16 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim } } - List list = this.fetchStatisticsFromSwitch(switchId, statType, - target); + List list = this.fetchStatisticsFromSwitch(switchId, statType, target); - return (list == null) ? null : - (statType == OFStatisticsType.VENDOR) ? v6StatsListToOFStatsList(list) : list; + return (statType == OFStatisticsType.VENDOR) ? v6StatsListToOFStatsList(list) : list; } @Override public List getOFDescStatistics(Long switchId) { - if (!descStatistics.containsKey(switchId)) - return this.dummyList; + if (!descStatistics.containsKey(switchId)) { + return Collections.emptyList(); + } return descStatistics.get(switchId); } @@ -863,7 +864,7 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim @Override public List getOFPortStatistics(Long switchId) { if (!portStatistics.containsKey(switchId)) { - return this.dummyList; + return Collections.emptyList(); } return portStatistics.get(switchId); @@ -872,7 +873,7 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim @Override public List getOFPortStatistics(Long switchId, short portId) { if (!portStatistics.containsKey(switchId)) { - return this.dummyList; + return Collections.emptyList(); } List list = new ArrayList(1); for (OFStatistics stats : portStatistics.get(switchId)) { @@ -887,7 +888,7 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim @Override public List getOFTableStatistics(Long switchId) { if (!tableStatistics.containsKey(switchId)) { - return this.dummyList; + return Collections.emptyList(); } return tableStatistics.get(switchId); @@ -896,7 +897,7 @@ public class OFStatisticsManager implements IOFStatisticsManager, IInventoryShim @Override public List getOFTableStatistics(Long switchId, Byte tableId) { if (!tableStatistics.containsKey(switchId)) { - return this.dummyList; + return Collections.emptyList(); } List list = new ArrayList(1); diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/PortStatisticsConverter.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/PortStatisticsConverter.java index b18eae971b..0fe1c72345 100644 --- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/PortStatisticsConverter.java +++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/PortStatisticsConverter.java @@ -9,6 +9,7 @@ package org.opendaylight.controller.protocol_plugin.openflow.internal; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.opendaylight.controller.sal.core.Node; @@ -36,7 +37,7 @@ public class PortStatisticsConverter { public PortStatisticsConverter(long switchId, List statsList) { this.switchId = switchId; if (statsList == null || statsList.isEmpty()) { - this.ofStatsList = new ArrayList(1); // dummy list + this.ofStatsList = Collections.emptyList(); } else { this.ofStatsList = new ArrayList(statsList); } diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadService.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadService.java index 215216e25f..f1f5944f37 100644 --- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadService.java +++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadService.java @@ -9,6 +9,7 @@ package org.opendaylight.controller.protocol_plugin.openflow.internal; +import java.util.Collections; import java.util.Dictionary; import java.util.List; import java.util.Set; @@ -122,12 +123,12 @@ public class ReadService implements IPluginInReadService, IReadFilterInternalLis public List readAllFlow(Node node, boolean cached) { if (!node.getType().equals(NodeIDType.OPENFLOW)) { logger.error("Invalid node type"); - return null; + return Collections.emptyList(); } if (!connectionOutService.isLocal(node)) { logger.debug("This Controller is not the master for the node : " + node); - return null; + return Collections.emptyList(); } return filter.readAllFlow(containerName, node, cached); @@ -170,12 +171,12 @@ public class ReadService implements IPluginInReadService, IReadFilterInternalLis boolean cached) { if (!node.getType().equals(NodeIDType.OPENFLOW)) { logger.error("Invalid node type"); - return null; + return Collections.emptyList(); } if (!connectionOutService.isLocal(node)) { logger.debug("This Controller is not the master for node : " + node); - return null; + return Collections.emptyList(); } return filter.readAllNodeConnector(containerName, node, cached); @@ -217,12 +218,12 @@ public class ReadService implements IPluginInReadService, IReadFilterInternalLis public List readAllNodeTable(Node node, boolean cached) { if (!node.getType().equals(NodeIDType.OPENFLOW)) { logger.error("Invalid node type"); - return null; + return Collections.emptyList(); } if (!connectionOutService.isLocal(node)) { logger.debug("This Controller is not the master for node : " + node); - return null; + return Collections.emptyList(); } return filter.readAllNodeTable(containerName, node, cached); diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadServiceFilter.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadServiceFilter.java index d7bf2e2258..b21a105f15 100644 --- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadServiceFilter.java +++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/ReadServiceFilter.java @@ -189,7 +189,7 @@ public class ReadServiceFilter implements IReadServiceFilter, IContainerListener List flowOnNodeList = new FlowStatisticsConverter(ofList).getFlowOnNodeList(node); List filteredList = filterFlowListPerContainer(container, node, flowOnNodeList); - return (filteredList == null || filteredList.isEmpty()) ? null : filteredList.get(0); + return (filteredList.isEmpty()) ? null : filteredList.get(0); } @Override @@ -203,10 +203,8 @@ public class ReadServiceFilter implements IReadServiceFilter, IContainerListener // Convert and filter the statistics per container List flowOnNodeList = new FlowStatisticsConverter(ofList).getFlowOnNodeList(node); - List filteredList = filterFlowListPerContainer(container, node, flowOnNodeList); - - return (filteredList == null) ? null : filteredList; + return filterFlowListPerContainer(container, node, flowOnNodeList); } @Override @@ -233,10 +231,10 @@ public class ReadServiceFilter implements IReadServiceFilter, IContainerListener * @param list * @return */ - public List filterFlowListPerContainer(String container, + private List filterFlowListPerContainer(String container, Node nodeId, List list) { if (list == null) { - return null; + return Collections.emptyList(); } // Create new filtered list of flows @@ -260,9 +258,9 @@ public class ReadServiceFilter implements IReadServiceFilter, IContainerListener * @param list * @return */ - public List filterPortListPerContainer(String container, long switchId, List list) { + private List filterPortListPerContainer(String container, long switchId, List list) { if (list == null) { - return null; + return Collections.emptyList(); } // Create new filtered list of flows @@ -281,10 +279,10 @@ public class ReadServiceFilter implements IReadServiceFilter, IContainerListener } - public List filterTableListPerContainer( + private List filterTableListPerContainer( String container, long switchId, List list) { if (list == null) { - return null; + return Collections.emptyList(); } // Create new filtered list of node tables diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/TableStatisticsConverter.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/TableStatisticsConverter.java index 993f8976fc..bcc2445808 100644 --- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/TableStatisticsConverter.java +++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/TableStatisticsConverter.java @@ -8,6 +8,7 @@ package org.opendaylight.controller.protocol_plugin.openflow.internal; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.opendaylight.controller.sal.core.Node; @@ -33,7 +34,7 @@ public class TableStatisticsConverter { public TableStatisticsConverter(long switchId, List statsList) { this.switchId = switchId; if (statsList == null || statsList.isEmpty()) { - this.ofStatsList = new ArrayList(1); // dummy list + this.ofStatsList = Collections.emptyList(); } else { this.ofStatsList = new ArrayList(statsList); } diff --git a/opendaylight/sal/implementation/src/main/java/org/opendaylight/controller/sal/implementation/internal/ReadService.java b/opendaylight/sal/implementation/src/main/java/org/opendaylight/controller/sal/implementation/internal/ReadService.java index bdd546055c..12de35f536 100644 --- a/opendaylight/sal/implementation/src/main/java/org/opendaylight/controller/sal/implementation/internal/ReadService.java +++ b/opendaylight/sal/implementation/src/main/java/org/opendaylight/controller/sal/implementation/internal/ReadService.java @@ -12,6 +12,7 @@ package org.opendaylight.controller.sal.implementation.internal; import java.net.InetAddress; import java.net.UnknownHostException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; @@ -201,7 +202,7 @@ public class ReadService implements IReadService, CommandProvider, IPluginOutRea } } logger.warn("Plugin {} unavailable", node.getType()); - return null; + return Collections.emptyList(); } @Override @@ -213,7 +214,7 @@ public class ReadService implements IReadService, CommandProvider, IPluginOutRea } } logger.warn("Plugin {} unavailable", node.getType()); - return null; + return Collections.emptyList(); } @Override @@ -276,7 +277,7 @@ public class ReadService implements IReadService, CommandProvider, IPluginOutRea } } logger.warn("Plugin {} unavailable", node.getType()); - return null; + return Collections.emptyList(); } @Override @@ -288,7 +289,7 @@ public class ReadService implements IReadService, CommandProvider, IPluginOutRea } } logger.warn("Plugin {} unavailable", node.getType()); - return null; + return Collections.emptyList(); } @@ -327,7 +328,7 @@ public class ReadService implements IReadService, CommandProvider, IPluginOutRea } } logger.warn("Plugin {} unavailable", node.getType()); - return null; + return Collections.emptyList(); } @Override diff --git a/opendaylight/statisticsmanager/implementation/src/main/java/org/opendaylight/controller/statisticsmanager/internal/StatisticsManager.java b/opendaylight/statisticsmanager/implementation/src/main/java/org/opendaylight/controller/statisticsmanager/internal/StatisticsManager.java index 92ed44efbb..aa6e4ac383 100644 --- a/opendaylight/statisticsmanager/implementation/src/main/java/org/opendaylight/controller/statisticsmanager/internal/StatisticsManager.java +++ b/opendaylight/statisticsmanager/implementation/src/main/java/org/opendaylight/controller/statisticsmanager/internal/StatisticsManager.java @@ -101,7 +101,6 @@ public class StatisticsManager implements IStatisticsManager, IReadServiceListen triggers = new ConcurrentHashMap(); } - @SuppressWarnings("deprecation") private void allocateCaches() { if (clusterContainerService == null) { nonClusterObjectCreate(); @@ -126,7 +125,7 @@ public class StatisticsManager implements IStatisticsManager, IReadServiceListen log.debug("Skipping statistics cache creation - already present"); } } - @SuppressWarnings({ "unchecked", "deprecation" }) + @SuppressWarnings({ "unchecked" }) private void retrieveCaches() { ConcurrentMap map; @@ -426,7 +425,7 @@ public class StatisticsManager implements IStatisticsManager, IReadServiceListen @Override public List getNodeConnectorStatistics(Node node) { if (node == null){ - return null; + return Collections.emptyList(); } List statList = new ArrayList(); @@ -456,7 +455,7 @@ public class StatisticsManager implements IStatisticsManager, IReadServiceListen @Override public List getNodeTableStatistics(Node node){ if (node == null){ - return null; + return Collections.emptyList(); } List statList = new ArrayList(); List cachedList = tableStatistics.get(node);