From b1548eeffaa9e025f466afe9be748fecf82ef765 Mon Sep 17 00:00:00 2001 From: Jian Li Date: Wed, 11 May 2016 09:49:57 -0700 Subject: [PATCH] Code clean up for control plane manager - Exception handlers should preserve the original exception - Collection.isEmpty() should be used to test for emptiness Change-Id: Ic76cf94f84fa761bb64b608df61fbd259c7990c4 --- .../cpman/cli/ResourceNameCompleter.java | 2 +- .../cpman/gui/CpmanViewMessageHandler.java | 19 ++++----------- .../cpman/impl/DefaultMetricsDatabase.java | 24 +++++++++---------- .../SystemMetricsCollectorWebResource.java | 18 +++++--------- 4 files changed, 23 insertions(+), 40 deletions(-) diff --git a/apps/cpman/app/src/main/java/org/onosproject/cpman/cli/ResourceNameCompleter.java b/apps/cpman/app/src/main/java/org/onosproject/cpman/cli/ResourceNameCompleter.java index 07e669ef2c..a87cf6e7eb 100644 --- a/apps/cpman/app/src/main/java/org/onosproject/cpman/cli/ResourceNameCompleter.java +++ b/apps/cpman/app/src/main/java/org/onosproject/cpman/cli/ResourceNameCompleter.java @@ -80,7 +80,7 @@ public class ResourceNameCompleter extends AbstractCompleter { SortedSet strings = delegate.getStrings(); - if (set.size() != 0) { + if (!set.isEmpty()) { set.forEach(strings::add); } } diff --git a/apps/cpman/app/src/main/java/org/onosproject/cpman/gui/CpmanViewMessageHandler.java b/apps/cpman/app/src/main/java/org/onosproject/cpman/gui/CpmanViewMessageHandler.java index a5f220d54e..58a8ba16c3 100644 --- a/apps/cpman/app/src/main/java/org/onosproject/cpman/gui/CpmanViewMessageHandler.java +++ b/apps/cpman/app/src/main/java/org/onosproject/cpman/gui/CpmanViewMessageHandler.java @@ -35,14 +35,11 @@ import org.onosproject.ui.RequestHandler; import org.onosproject.ui.UiMessageHandler; import org.onosproject.ui.chart.ChartModel; import org.onosproject.ui.chart.ChartRequestHandler; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.Collection; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.stream.LongStream; @@ -54,8 +51,6 @@ import static org.onosproject.cpman.ControlResource.Type.CONTROL_MESSAGE; */ public class CpmanViewMessageHandler extends UiMessageHandler { - private final Logger log = LoggerFactory.getLogger(getClass()); - private static final String CPMAN_DATA_REQ = "cpmanDataRequest"; private static final String CPMAN_DATA_RESP = "cpmanDataResponse"; private static final String CPMANS = "cpmans"; @@ -131,16 +126,10 @@ public class CpmanViewMessageHandler extends UiMessageHandler { ClusterService cs, DeviceId deviceId) { Map data = Maps.newHashMap(); for (ControlMetricType cmt : CONTROL_MESSAGE_METRICS) { - ControlLoadSnapshot cls; - try { - cls = cpms.getLoad(cs.getLocalNode().id(), - cmt, NUM_OF_DATA_POINTS, TimeUnit.MINUTES, - Optional.of(deviceId)).get(); - data.put(cmt, Math.round(LongStream.of(cls.recent()).average().getAsDouble())); - timestamp = cls.time(); - } catch (InterruptedException | ExecutionException e) { - log.warn(e.getMessage()); - } + ControlLoadSnapshot cls = cpms.getLoadSync(cs.getLocalNode().id(), + cmt, NUM_OF_DATA_POINTS, TimeUnit.MINUTES, Optional.of(deviceId)); + data.put(cmt, Math.round(LongStream.of(cls.recent()).average().getAsDouble())); + timestamp = cls.time(); } return data; } diff --git a/apps/cpman/app/src/main/java/org/onosproject/cpman/impl/DefaultMetricsDatabase.java b/apps/cpman/app/src/main/java/org/onosproject/cpman/impl/DefaultMetricsDatabase.java index 1edf159bff..6ff46ba811 100644 --- a/apps/cpman/app/src/main/java/org/onosproject/cpman/impl/DefaultMetricsDatabase.java +++ b/apps/cpman/app/src/main/java/org/onosproject/cpman/impl/DefaultMetricsDatabase.java @@ -94,7 +94,7 @@ public final class DefaultMetricsDatabase implements MetricsDatabase { sample.setValue(metricType, value); sample.update(); } catch (IOException e) { - log.error("Failed to update metric value due to {}", e.getMessage()); + log.error("Failed to update metric value due to {}", e); } } @@ -112,12 +112,12 @@ public final class DefaultMetricsDatabase implements MetricsDatabase { checkArgument(rrdDb.containsDs(k), NON_EXIST_METRIC); sample.setValue(k, v); } catch (IOException e) { - log.error("Failed to update metric value due to {}", e.getMessage()); + log.error("Failed to update metric value due to {}", e); } }); sample.update(); } catch (IOException e) { - log.error("Failed to update metric values due to {}", e.getMessage()); + log.error("Failed to update metric values due to {}", e); } } @@ -127,7 +127,7 @@ public final class DefaultMetricsDatabase implements MetricsDatabase { checkArgument(rrdDb.containsDs(metricType), NON_EXIST_METRIC); return rrdDb.getDatasource(metricType).getLastValue(); } catch (IOException e) { - log.error("Failed to obtain metric value due to {}", e.getMessage()); + log.error("Failed to obtain metric value due to {}", e); return 0D; } } @@ -146,7 +146,7 @@ public final class DefaultMetricsDatabase implements MetricsDatabase { return new double[0]; } } catch (IOException e) { - log.error("Failed to obtain metric values due to {}", e.getMessage()); + log.error("Failed to obtain metric values due to {}", e); return new double[0]; } } @@ -159,7 +159,7 @@ public final class DefaultMetricsDatabase implements MetricsDatabase { long startTime = endTime - SECONDS_OF_DAY + 1; return minMetric(metricType, startTime, endTime); } catch (IOException e) { - log.error("Failed to obtain metric value due to {}", e.getMessage()); + log.error("Failed to obtain metric value due to {}", e); return 0D; } } @@ -172,7 +172,7 @@ public final class DefaultMetricsDatabase implements MetricsDatabase { long startTime = endTime - SECONDS_OF_DAY; return maxMetric(metricType, startTime, endTime); } catch (IOException e) { - log.error("Failed to obtain metric value due to {}", e.getMessage()); + log.error("Failed to obtain metric value due to {}", e); return 0D; } } @@ -185,7 +185,7 @@ public final class DefaultMetricsDatabase implements MetricsDatabase { long startTime = endTime - SECONDS_OF_DAY; return metrics(metricType, startTime, endTime); } catch (IOException e) { - log.error("Failed to obtain metric values due to {}", e.getMessage()); + log.error("Failed to obtain metric values due to {}", e); return new double[0]; } } @@ -202,7 +202,7 @@ public final class DefaultMetricsDatabase implements MetricsDatabase { return new double[0]; } } catch (IOException e) { - log.error("Failed to obtain metric values due to {}", e.getMessage()); + log.error("Failed to obtain metric values due to {}", e); return new double[0]; } } @@ -213,7 +213,7 @@ public final class DefaultMetricsDatabase implements MetricsDatabase { checkArgument(rrdDb.containsDs(metricType), NON_EXIST_METRIC); return rrdDb.getLastUpdateTime(); } catch (IOException e) { - log.error("Failed to obtain last update time due to {}", e.getMessage()); + log.error("Failed to obtain last update time due to {}", e); return 0L; } } @@ -296,7 +296,7 @@ public final class DefaultMetricsDatabase implements MetricsDatabase { public MetricsDatabase build() { checkNotNull(metricName, METRIC_NAME_MSG); checkNotNull(resourceName, RESOURCE_NAME_MSG); - checkArgument(dsDefs.size() != 0, METRIC_TYPE_MSG); + checkArgument(!dsDefs.isEmpty(), METRIC_TYPE_MSG); // define the resolution of monitored metrics rrdDef = new RrdDef(DB_PATH + SPLITTER + metricName + @@ -317,7 +317,7 @@ public final class DefaultMetricsDatabase implements MetricsDatabase { // always store the metric data in memory... rrdDb = new RrdDb(rrdDef, RrdBackendFactory.getFactory(STORING_METHOD)); } catch (IOException e) { - log.warn("Failed to create a new round-robin database due to {}", e.getMessage()); + log.warn("Failed to create a new round-robin database due to {}", e); } return new DefaultMetricsDatabase(metricName, resourceName, rrdDb); diff --git a/apps/cpman/app/src/main/java/org/onosproject/cpman/rest/SystemMetricsCollectorWebResource.java b/apps/cpman/app/src/main/java/org/onosproject/cpman/rest/SystemMetricsCollectorWebResource.java index 5dd941395c..37d39cb7e4 100644 --- a/apps/cpman/app/src/main/java/org/onosproject/cpman/rest/SystemMetricsCollectorWebResource.java +++ b/apps/cpman/app/src/main/java/org/onosproject/cpman/rest/SystemMetricsCollectorWebResource.java @@ -27,8 +27,8 @@ import org.onosproject.cpman.ControlResource; import org.onosproject.cpman.MetricValue; import org.onosproject.cpman.SystemInfo; import org.onosproject.cpman.impl.DefaultSystemInfo; -import org.onosproject.cpman.impl.SystemMetricsAggregator; import org.onosproject.cpman.impl.SystemInfoFactory; +import org.onosproject.cpman.impl.SystemMetricsAggregator; import org.onosproject.rest.AbstractWebResource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -36,7 +36,6 @@ import org.slf4j.LoggerFactory; import javax.ws.rs.Consumes; import javax.ws.rs.POST; import javax.ws.rs.Path; -import javax.ws.rs.Produces; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import java.io.IOException; @@ -87,7 +86,6 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { @POST @Path("cpu_metrics") @Consumes(MediaType.APPLICATION_JSON) - @Produces(MediaType.APPLICATION_JSON) public Response cpuMetrics(InputStream stream) { ObjectNode root = mapper().createObjectNode(); ControlMetric cm; @@ -134,7 +132,7 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { aggregator.increment(ControlMetricType.CPU_IDLE_TIME, cpuIdleTime); } catch (IOException e) { - throw new IllegalArgumentException(e.getMessage()); + throw new IllegalArgumentException(e); } return ok(root).build(); } @@ -149,7 +147,6 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { @POST @Path("memory_metrics") @Consumes(MediaType.APPLICATION_JSON) - @Produces(MediaType.APPLICATION_JSON) public Response memoryMetrics(InputStream stream) { ObjectNode root = mapper().createObjectNode(); ControlMetric cm; @@ -190,7 +187,7 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { aggregator.increment(ControlMetricType.MEMORY_FREE, memFree); } catch (IOException e) { - throw new IllegalArgumentException(e.getMessage()); + throw new IllegalArgumentException(e); } return ok(root).build(); } @@ -205,7 +202,6 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { @POST @Path("disk_metrics") @Consumes(MediaType.APPLICATION_JSON) - @Produces(MediaType.APPLICATION_JSON) public Response diskMetrics(InputStream stream) { ObjectNode root = mapper().createObjectNode(); ControlMetric cm; @@ -237,7 +233,7 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { ControlMetricType.DISK_WRITE_BYTES, writeBytes); } } catch (IOException e) { - throw new IllegalArgumentException(e.getMessage()); + throw new IllegalArgumentException(e); } return ok(root).build(); } @@ -252,7 +248,6 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { @POST @Path("network_metrics") @Consumes(MediaType.APPLICATION_JSON) - @Produces(MediaType.APPLICATION_JSON) public Response networkMetrics(InputStream stream) { ObjectNode root = mapper().createObjectNode(); ControlMetric cm; @@ -299,7 +294,7 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { ControlMetricType.NW_OUTGOING_PACKETS, outPackets); } } catch (IOException e) { - throw new IllegalArgumentException(e.getMessage()); + throw new IllegalArgumentException(e); } return ok(root).build(); } @@ -316,7 +311,6 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { @POST @Path("system_info") @Consumes(MediaType.APPLICATION_JSON) - @Produces(MediaType.APPLICATION_JSON) public Response systemInfo(InputStream stream) { ObjectNode root = mapper().createObjectNode(); @@ -343,7 +337,7 @@ public class SystemMetricsCollectorWebResource extends AbstractWebResource { } } catch (IOException e) { - throw new IllegalArgumentException(e.getMessage()); + throw new IllegalArgumentException(e); } return ok(root).build(); }