홍길동 / onos

Go to a project
Toggle navigation Toggle navigation pinning
Jian Li
Committed by Gerrit Code Review
1fdd2242 1 parent e75cf18f

Not allow null value for all control metrics, revise unit test 

Current implementation allows null value from REST API.
Null metric value leads the metrics database to not store the value
properly. This commit tries to fix this issue.

Change-Id: I82a5fd0d11671db4d2136a3fc40090b7684ba91a
Inline Side-by-side
Showing 2 changed files with 80 additions and 94 deletions
...@@ -44,9 +44,10 @@ import static org.onlab.util.Tools.nullIsIllegal; ...@@ -44,9 +44,10 @@ import static org.onlab.util.Tools.nullIsIllegal;
44 public class ControlMetricsCollectorWebResource extends AbstractWebResource { 44 public class ControlMetricsCollectorWebResource extends AbstractWebResource {
45 45
46 final ControlPlaneMonitorService service = get(ControlPlaneMonitorService.class); 46 final ControlPlaneMonitorService service = get(ControlPlaneMonitorService.class);
47 - public static final int UPDATE_INTERVAL = 1; // 1 minute update interval 47 + public static final int UPDATE_INTERVAL_IN_MINUTE = 1;
48 public static final String INVALID_SYSTEM_SPECS = "Invalid system specifications"; 48 public static final String INVALID_SYSTEM_SPECS = "Invalid system specifications";
49 public static final String INVALID_RESOURCE_NAME = "Invalid resource name"; 49 public static final String INVALID_RESOURCE_NAME = "Invalid resource name";
50 + public static final String INVALID_REQUEST = "Invalid request";
50 51
51 /** 52 /**
52 * Collects CPU metrics. 53 * Collects CPU metrics.
...@@ -64,41 +65,31 @@ public class ControlMetricsCollectorWebResource extends AbstractWebResource { ...@@ -64,41 +65,31 @@ public class ControlMetricsCollectorWebResource extends AbstractWebResource {
64 ControlMetric cm; 65 ControlMetric cm;
65 try { 66 try {
66 ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream); 67 ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream);
67 - JsonNode cpuLoadJson = jsonTree.get("cpuLoad"); 68 + long cpuLoad = nullIsIllegal(jsonTree.get("cpuLoad").asLong(), INVALID_REQUEST);
68 - JsonNode totalCpuTimeJson = jsonTree.get("totalCpuTime"); 69 + long totalCpuTime = nullIsIllegal(jsonTree.get("totalCpuTime").asLong(), INVALID_REQUEST);
69 - JsonNode sysCpuTimeJson = jsonTree.get("sysCpuTime"); 70 + long sysCpuTime = nullIsIllegal(jsonTree.get("sysCpuTime").asLong(), INVALID_REQUEST);
70 - JsonNode userCpuTimeJson = jsonTree.get("userCpuTime"); 71 + long userCpuTime = nullIsIllegal(jsonTree.get("userCpuTime").asLong(), INVALID_REQUEST);
71 - JsonNode cpuIdleTimeJson = jsonTree.get("cpuIdleTime"); 72 + long cpuIdleTime = nullIsIllegal(jsonTree.get("cpuIdleTime").asLong(), INVALID_REQUEST);
72 73
73 - if (cpuLoadJson != null) {
74 cm = new ControlMetric(ControlMetricType.CPU_LOAD, 74 cm = new ControlMetric(ControlMetricType.CPU_LOAD,
75 - new MetricValue.Builder().load(cpuLoadJson.asLong()).add()); 75 + new MetricValue.Builder().load(cpuLoad).add());
76 - service.updateMetric(cm, UPDATE_INTERVAL, Optional.ofNullable(null)); 76 + service.updateMetric(cm, UPDATE_INTERVAL_IN_MINUTE, Optional.ofNullable(null));
77 - }
78 77
79 - if (totalCpuTimeJson != null) {
80 cm = new ControlMetric(ControlMetricType.TOTAL_CPU_TIME, 78 cm = new ControlMetric(ControlMetricType.TOTAL_CPU_TIME,
81 - new MetricValue.Builder().load(totalCpuTimeJson.asLong()).add()); 79 + new MetricValue.Builder().load(totalCpuTime).add());
82 - service.updateMetric(cm, UPDATE_INTERVAL, Optional.ofNullable(null)); 80 + service.updateMetric(cm, UPDATE_INTERVAL_IN_MINUTE, Optional.ofNullable(null));
83 - }
84 81
85 - if (sysCpuTimeJson != null) {
86 cm = new ControlMetric(ControlMetricType.SYS_CPU_TIME, 82 cm = new ControlMetric(ControlMetricType.SYS_CPU_TIME,
87 - new MetricValue.Builder().load(sysCpuTimeJson.asLong()).add()); 83 + new MetricValue.Builder().load(sysCpuTime).add());
88 - service.updateMetric(cm, UPDATE_INTERVAL, Optional.ofNullable(null)); 84 + service.updateMetric(cm, UPDATE_INTERVAL_IN_MINUTE, Optional.ofNullable(null));
89 - }
90 85
91 - if (userCpuTimeJson != null) {
92 cm = new ControlMetric(ControlMetricType.USER_CPU_TIME, 86 cm = new ControlMetric(ControlMetricType.USER_CPU_TIME,
93 - new MetricValue.Builder().load(userCpuTimeJson.asLong()).add()); 87 + new MetricValue.Builder().load(userCpuTime).add());
94 - service.updateMetric(cm, UPDATE_INTERVAL, Optional.ofNullable(null)); 88 + service.updateMetric(cm, UPDATE_INTERVAL_IN_MINUTE, Optional.ofNullable(null));
95 - }
96 89
97 - if (cpuIdleTimeJson != null) {
98 cm = new ControlMetric(ControlMetricType.CPU_IDLE_TIME, 90 cm = new ControlMetric(ControlMetricType.CPU_IDLE_TIME,
99 - new MetricValue.Builder().load(cpuIdleTimeJson.asLong()).add()); 91 + new MetricValue.Builder().load(cpuIdleTime).add());
100 - service.updateMetric(cm, UPDATE_INTERVAL, Optional.ofNullable(null)); 92 + service.updateMetric(cm, UPDATE_INTERVAL_IN_MINUTE, Optional.ofNullable(null));
101 - }
102 93
103 } catch (IOException e) { 94 } catch (IOException e) {
104 throw new IllegalArgumentException(e.getMessage()); 95 throw new IllegalArgumentException(e.getMessage());
...@@ -122,34 +113,26 @@ public class ControlMetricsCollectorWebResource extends AbstractWebResource { ...@@ -122,34 +113,26 @@ public class ControlMetricsCollectorWebResource extends AbstractWebResource {
122 ControlMetric cm; 113 ControlMetric cm;
123 try { 114 try {
124 ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream); 115 ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream);
125 - JsonNode memUsedRatio = jsonTree.get("memoryUsedRatio"); 116 + long memUsedRatio = nullIsIllegal(jsonTree.get("memoryUsedRatio").asLong(), INVALID_REQUEST);
126 - JsonNode memFreeRatio = jsonTree.get("memoryFreeRatio"); 117 + long memFreeRatio = nullIsIllegal(jsonTree.get("memoryFreeRatio").asLong(), INVALID_REQUEST);
127 - JsonNode memUsed = jsonTree.get("memoryUsed"); 118 + long memUsed = nullIsIllegal(jsonTree.get("memoryUsed").asLong(), INVALID_REQUEST);
128 - JsonNode memFree = jsonTree.get("memoryFree"); 119 + long memFree = nullIsIllegal(jsonTree.get("memoryFree").asLong(), INVALID_REQUEST);
129 120
130 - if (memUsedRatio != null) {
131 cm = new ControlMetric(ControlMetricType.MEMORY_USED_RATIO, 121 cm = new ControlMetric(ControlMetricType.MEMORY_USED_RATIO,
132 - new MetricValue.Builder().load(memUsedRatio.asLong()).add()); 122 + new MetricValue.Builder().load(memUsedRatio).add());
133 - service.updateMetric(cm, UPDATE_INTERVAL, Optional.ofNullable(null)); 123 + service.updateMetric(cm, UPDATE_INTERVAL_IN_MINUTE, Optional.ofNullable(null));
134 - }
135 124
136 - if (memFreeRatio != null) {
137 cm = new ControlMetric(ControlMetricType.MEMORY_FREE_RATIO, 125 cm = new ControlMetric(ControlMetricType.MEMORY_FREE_RATIO,
138 - new MetricValue.Builder().load(memFreeRatio.asLong()).add()); 126 + new MetricValue.Builder().load(memFreeRatio).add());
139 - service.updateMetric(cm, UPDATE_INTERVAL, Optional.ofNullable(null)); 127 + service.updateMetric(cm, UPDATE_INTERVAL_IN_MINUTE, Optional.ofNullable(null));
140 - }
141 128
142 - if (memUsed != null) {
143 cm = new ControlMetric(ControlMetricType.MEMORY_USED, 129 cm = new ControlMetric(ControlMetricType.MEMORY_USED,
144 - new MetricValue.Builder().load(memUsed.asLong()).add()); 130 + new MetricValue.Builder().load(memUsed).add());
145 - service.updateMetric(cm, UPDATE_INTERVAL, Optional.ofNullable(null)); 131 + service.updateMetric(cm, UPDATE_INTERVAL_IN_MINUTE, Optional.ofNullable(null));
146 - }
147 132
148 - if (memFree != null) {
149 cm = new ControlMetric(ControlMetricType.MEMORY_FREE, 133 cm = new ControlMetric(ControlMetricType.MEMORY_FREE,
150 - new MetricValue.Builder().load(memFree.asLong()).add()); 134 + new MetricValue.Builder().load(memFree).add());
151 - service.updateMetric(cm, UPDATE_INTERVAL, Optional.ofNullable(null)); 135 + service.updateMetric(cm, UPDATE_INTERVAL_IN_MINUTE, Optional.ofNullable(null));
152 - }
153 136
154 } catch (IOException e) { 137 } catch (IOException e) {
155 throw new IllegalArgumentException(e.getMessage()); 138 throw new IllegalArgumentException(e.getMessage());
...@@ -170,29 +153,25 @@ public class ControlMetricsCollectorWebResource extends AbstractWebResource { ...@@ -170,29 +153,25 @@ public class ControlMetricsCollectorWebResource extends AbstractWebResource {
170 @Produces(MediaType.APPLICATION_JSON) 153 @Produces(MediaType.APPLICATION_JSON)
171 public Response diskMetrics(InputStream stream) { 154 public Response diskMetrics(InputStream stream) {
172 ObjectNode root = mapper().createObjectNode(); 155 ObjectNode root = mapper().createObjectNode();
173 - final ControlMetric[] cm = new ControlMetric[1]; 156 + ControlMetric cm;
174 try { 157 try {
175 ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream); 158 ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream);
176 ArrayNode diskRes = (ArrayNode) jsonTree.get("disks"); 159 ArrayNode diskRes = (ArrayNode) jsonTree.get("disks");
177 - diskRes.forEach(node-> { 160 + for (JsonNode node : diskRes) {
178 JsonNode resourceName = node.get("resourceName"); 161 JsonNode resourceName = node.get("resourceName");
179 nullIsIllegal(resourceName, INVALID_RESOURCE_NAME); 162 nullIsIllegal(resourceName, INVALID_RESOURCE_NAME);
180 163
181 - JsonNode readBytes = jsonTree.get("readBytes"); 164 + long readBytes = nullIsIllegal(node.get("readBytes").asLong(), INVALID_REQUEST);
182 - JsonNode writeBytes = jsonTree.get("writeBytes"); 165 + long writeBytes = nullIsIllegal(node.get("writeBytes").asLong(), INVALID_REQUEST);
183 166
184 - if (readBytes != null) { 167 + cm = new ControlMetric(ControlMetricType.DISK_READ_BYTES,
185 - cm[0] = new ControlMetric(ControlMetricType.DISK_READ_BYTES, 168 + new MetricValue.Builder().load(readBytes).add());
186 - new MetricValue.Builder().load(readBytes.asLong()).add()); 169 + service.updateMetric(cm, UPDATE_INTERVAL_IN_MINUTE, resourceName.asText());
187 - service.updateMetric(cm[0], UPDATE_INTERVAL, resourceName.asText());
188 - }
189 170
190 - if (writeBytes != null) { 171 + cm = new ControlMetric(ControlMetricType.DISK_WRITE_BYTES,
191 - cm[0] = new ControlMetric(ControlMetricType.DISK_WRITE_BYTES, 172 + new MetricValue.Builder().load(writeBytes).add());
192 - new MetricValue.Builder().load(writeBytes.asLong()).add()); 173 + service.updateMetric(cm, UPDATE_INTERVAL_IN_MINUTE, resourceName.asText());
193 - service.updateMetric(cm[0], UPDATE_INTERVAL, resourceName.asText());
194 } 174 }
195 - });
196 } catch (IOException e) { 175 } catch (IOException e) {
197 throw new IllegalArgumentException(e.getMessage()); 176 throw new IllegalArgumentException(e.getMessage());
198 } 177 }
...@@ -212,43 +191,35 @@ public class ControlMetricsCollectorWebResource extends AbstractWebResource { ...@@ -212,43 +191,35 @@ public class ControlMetricsCollectorWebResource extends AbstractWebResource {
212 @Produces(MediaType.APPLICATION_JSON) 191 @Produces(MediaType.APPLICATION_JSON)
213 public Response networkMetrics(InputStream stream) { 192 public Response networkMetrics(InputStream stream) {
214 ObjectNode root = mapper().createObjectNode(); 193 ObjectNode root = mapper().createObjectNode();
215 - final ControlMetric[] cm = new ControlMetric[1]; 194 + ControlMetric cm;
216 try { 195 try {
217 ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream); 196 ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream);
218 ArrayNode networkRes = (ArrayNode) jsonTree.get("networks"); 197 ArrayNode networkRes = (ArrayNode) jsonTree.get("networks");
219 - networkRes.forEach(node -> { 198 + for (JsonNode node : networkRes) {
220 JsonNode resourceName = node.get("resourceName"); 199 JsonNode resourceName = node.get("resourceName");
221 nullIsIllegal(resourceName, INVALID_RESOURCE_NAME); 200 nullIsIllegal(resourceName, INVALID_RESOURCE_NAME);
222 201
223 - JsonNode inBytes = jsonTree.get("incomingBytes"); 202 + long inBytes = nullIsIllegal(node.get("incomingBytes").asLong(), INVALID_REQUEST);
224 - JsonNode outBytes = jsonTree.get("outgoingBytes"); 203 + long outBytes = nullIsIllegal(node.get("outgoingBytes").asLong(), INVALID_REQUEST);
225 - JsonNode inPackets = jsonTree.get("incomingPackets"); 204 + long inPackets = nullIsIllegal(node.get("incomingPackets").asLong(), INVALID_REQUEST);
226 - JsonNode outPackets = jsonTree.get("outgoingPackets"); 205 + long outPackets = nullIsIllegal(node.get("outgoingPackets").asLong(), INVALID_REQUEST);
227 206
228 - if (inBytes != null) { 207 + cm = new ControlMetric(ControlMetricType.NW_INCOMING_BYTES,
229 - cm[0] = new ControlMetric(ControlMetricType.NW_INCOMING_BYTES, 208 + new MetricValue.Builder().load(inBytes).add());
230 - new MetricValue.Builder().load(inBytes.asLong()).add()); 209 + service.updateMetric(cm, UPDATE_INTERVAL_IN_MINUTE, resourceName.asText());
231 - service.updateMetric(cm[0], UPDATE_INTERVAL, resourceName.asText());
232 - }
233 210
234 - if (outBytes != null) { 211 + cm = new ControlMetric(ControlMetricType.NW_OUTGOING_BYTES,
235 - cm[0] = new ControlMetric(ControlMetricType.NW_OUTGOING_BYTES, 212 + new MetricValue.Builder().load(outBytes).add());
236 - new MetricValue.Builder().load(outBytes.asLong()).add()); 213 + service.updateMetric(cm, UPDATE_INTERVAL_IN_MINUTE, resourceName.asText());
237 - service.updateMetric(cm[0], UPDATE_INTERVAL, resourceName.asText());
238 - }
239 214
240 - if (inPackets != null) { 215 + cm = new ControlMetric(ControlMetricType.NW_INCOMING_PACKETS,
241 - cm[0] = new ControlMetric(ControlMetricType.NW_INCOMING_PACKETS, 216 + new MetricValue.Builder().load(inPackets).add());
242 - new MetricValue.Builder().load(inPackets.asLong()).add()); 217 + service.updateMetric(cm, UPDATE_INTERVAL_IN_MINUTE, resourceName.asText());
243 - service.updateMetric(cm[0], UPDATE_INTERVAL, resourceName.asText());
244 - }
245 218
246 - if (outPackets != null) { 219 + cm = new ControlMetric(ControlMetricType.NW_OUTGOING_PACKETS,
247 - cm[0] = new ControlMetric(ControlMetricType.NW_OUTGOING_PACKETS, 220 + new MetricValue.Builder().load(outPackets).add());
248 - new MetricValue.Builder().load(outPackets.asLong()).add()); 221 + service.updateMetric(cm, UPDATE_INTERVAL_IN_MINUTE, resourceName.asText());
249 - service.updateMetric(cm[0], UPDATE_INTERVAL, resourceName.asText());
250 } 222 }
251 - });
252 } catch (IOException e) { 223 } catch (IOException e) {
253 throw new IllegalArgumentException(e.getMessage()); 224 throw new IllegalArgumentException(e.getMessage());
254 } 225 }
......
...@@ -57,6 +57,15 @@ public class ControlMetricsCollectorResourceTest extends JerseyTest { ...@@ -57,6 +57,15 @@ public class ControlMetricsCollectorResourceTest extends JerseyTest {
57 private static final String PREFIX = "collector"; 57 private static final String PREFIX = "collector";
58 58
59 /** 59 /**
60 + * Constructs a control metrics collector resource test instance.
61 + */
62 + public ControlMetricsCollectorResourceTest() {
63 + super(new WebAppDescriptor.Builder("javax.ws.rs.Application",
64 + CPManWebApplication.class.getCanonicalName())
65 + .servletClass(ServletContainer.class).build());
66 + }
67 +
68 + /**
60 * Sets up the global values for all the tests. 69 * Sets up the global values for all the tests.
61 */ 70 */
62 @Before 71 @Before
...@@ -67,6 +76,9 @@ public class ControlMetricsCollectorResourceTest extends JerseyTest { ...@@ -67,6 +76,9 @@ public class ControlMetricsCollectorResourceTest extends JerseyTest {
67 BaseResource.setServiceDirectory(testDirectory); 76 BaseResource.setServiceDirectory(testDirectory);
68 } 77 }
69 78
79 + /**
80 + * Tests CPU metrics POST through REST API.
81 + */
70 @Test 82 @Test
71 public void testCpuMetricsPost() { 83 public void testCpuMetricsPost() {
72 mockControlPlaneMonitorService.updateMetric(anyObject(), anyInt(), 84 mockControlPlaneMonitorService.updateMetric(anyObject(), anyInt(),
...@@ -76,6 +88,9 @@ public class ControlMetricsCollectorResourceTest extends JerseyTest { ...@@ -76,6 +88,9 @@ public class ControlMetricsCollectorResourceTest extends JerseyTest {
76 basePostTest("cpu-metrics-post.json", PREFIX + "/cpu_metrics"); 88 basePostTest("cpu-metrics-post.json", PREFIX + "/cpu_metrics");
77 } 89 }
78 90
91 + /**
92 + * Tests memory metrics POST through REST API.
93 + */
79 @Test 94 @Test
80 public void testMemoryMetricsPost() { 95 public void testMemoryMetricsPost() {
81 mockControlPlaneMonitorService.updateMetric(anyObject(), anyInt(), 96 mockControlPlaneMonitorService.updateMetric(anyObject(), anyInt(),
...@@ -85,16 +100,22 @@ public class ControlMetricsCollectorResourceTest extends JerseyTest { ...@@ -85,16 +100,22 @@ public class ControlMetricsCollectorResourceTest extends JerseyTest {
85 basePostTest("memory-metrics-post.json", PREFIX + "/memory_metrics"); 100 basePostTest("memory-metrics-post.json", PREFIX + "/memory_metrics");
86 } 101 }
87 102
103 + /**
104 + * Tests disk metrics POST through REST API.
105 + */
88 @Test 106 @Test
89 - public void testDiskMetricsWithNullName() { 107 + public void testDiskMetrics() {
90 mockControlPlaneMonitorService.updateMetric(anyObject(), anyInt(), anyString()); 108 mockControlPlaneMonitorService.updateMetric(anyObject(), anyInt(), anyString());
91 expectLastCall().times(4); 109 expectLastCall().times(4);
92 replay(mockControlPlaneMonitorService); 110 replay(mockControlPlaneMonitorService);
93 basePostTest("disk-metrics-post.json", PREFIX + "/disk_metrics"); 111 basePostTest("disk-metrics-post.json", PREFIX + "/disk_metrics");
94 } 112 }
95 113
114 + /**
115 + * Tests network metrics POST through REST API.
116 + */
96 @Test 117 @Test
97 - public void testNetworkMetricsWithNullName() { 118 + public void testNetworkMetrics() {
98 mockControlPlaneMonitorService.updateMetric(anyObject(), anyInt(), anyString()); 119 mockControlPlaneMonitorService.updateMetric(anyObject(), anyInt(), anyString());
99 expectLastCall().times(8); 120 expectLastCall().times(8);
100 replay(mockControlPlaneMonitorService); 121 replay(mockControlPlaneMonitorService);
...@@ -123,12 +144,6 @@ public class ControlMetricsCollectorResourceTest extends JerseyTest { ...@@ -123,12 +144,6 @@ public class ControlMetricsCollectorResourceTest extends JerseyTest {
123 assertThat(response.getStatus(), is(HttpURLConnection.HTTP_OK)); 144 assertThat(response.getStatus(), is(HttpURLConnection.HTTP_OK));
124 } 145 }
125 146
126 - public ControlMetricsCollectorResourceTest() {
127 - super(new WebAppDescriptor.Builder("javax.ws.rs.Application",
128 - CPManWebApplication.class.getCanonicalName())
129 - .servletClass(ServletContainer.class).build());
130 - }
131 -
132 /** 147 /**
133 * Assigns an available port for the test. 148 * Assigns an available port for the test.
134 * 149 *
......