Cleanups to REST code suggested by Sonar
Change-Id: Ia399da70e7cd140514e07b63d9b2965fe86bbce7
Showing
5 changed files
with
30 additions
and
37 deletions
... | @@ -142,21 +142,21 @@ public class FlowsWebResource extends AbstractWebResource { | ... | @@ -142,21 +142,21 @@ public class FlowsWebResource extends AbstractWebResource { |
142 | InputStream stream) { | 142 | InputStream stream) { |
143 | URI location; | 143 | URI location; |
144 | try { | 144 | try { |
145 | - FlowRuleService service = get(FlowRuleService.class); | 145 | + ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream); |
146 | - ObjectNode root = (ObjectNode) mapper().readTree(stream); | 146 | + JsonNode specifiedDeviceId = jsonTree.get("deviceId"); |
147 | - JsonNode specifiedDeviceId = root.get("deviceId"); | ||
148 | if (specifiedDeviceId != null && | 147 | if (specifiedDeviceId != null && |
149 | !specifiedDeviceId.asText().equals(deviceId)) { | 148 | !specifiedDeviceId.asText().equals(deviceId)) { |
150 | throw new IllegalArgumentException( | 149 | throw new IllegalArgumentException( |
151 | "Invalid deviceId in flow creation request"); | 150 | "Invalid deviceId in flow creation request"); |
152 | } | 151 | } |
153 | - root.put("deviceId", deviceId); | 152 | + jsonTree.put("deviceId", deviceId); |
154 | - FlowRule rule = codec(FlowRule.class).decode(root, this); | 153 | + FlowRule rule = codec(FlowRule.class).decode(jsonTree, this); |
155 | service.applyFlowRules(rule); | 154 | service.applyFlowRules(rule); |
156 | location = new URI(Long.toString(rule.id().value())); | 155 | location = new URI(Long.toString(rule.id().value())); |
157 | } catch (IOException | URISyntaxException ex) { | 156 | } catch (IOException | URISyntaxException ex) { |
158 | - return Response.status(Response.Status.INTERNAL_SERVER_ERROR).build(); | 157 | + throw new IllegalArgumentException(ex); |
159 | } | 158 | } |
159 | + | ||
160 | return Response | 160 | return Response |
161 | .created(location) | 161 | .created(location) |
162 | .build(); | 162 | .build(); | ... | ... |
... | @@ -125,7 +125,7 @@ public class HostsWebResource extends AbstractWebResource { | ... | @@ -125,7 +125,7 @@ public class HostsWebResource extends AbstractWebResource { |
125 | hostProviderRegistry.unregister(hostProvider); | 125 | hostProviderRegistry.unregister(hostProvider); |
126 | 126 | ||
127 | } catch (IOException ex) { | 127 | } catch (IOException ex) { |
128 | - return Response.status(Response.Status.INTERNAL_SERVER_ERROR).build(); | 128 | + throw new IllegalArgumentException(ex); |
129 | } | 129 | } |
130 | return Response | 130 | return Response |
131 | .created(location) | 131 | .created(location) |
... | @@ -137,19 +137,20 @@ public class HostsWebResource extends AbstractWebResource { | ... | @@ -137,19 +137,20 @@ public class HostsWebResource extends AbstractWebResource { |
137 | new ProviderId("host", "org.onosproject.rest", true); | 137 | new ProviderId("host", "org.onosproject.rest", true); |
138 | private HostProviderService hostProviderService; | 138 | private HostProviderService hostProviderService; |
139 | 139 | ||
140 | - // Not implemented since there is no need to check for hosts on network | 140 | + private InternalHostProvider() { |
141 | - public void triggerProbe(Host host) { | ||
142 | } | 141 | } |
143 | 142 | ||
144 | - // Creates new InternalHostProvider with a HostProviderRegistry param. | 143 | + public void triggerProbe(Host host) { |
145 | - private InternalHostProvider() { | 144 | + // Not implemented since there is no need to check for hosts on network |
146 | } | 145 | } |
147 | 146 | ||
148 | public void setHostProviderService(HostProviderService service) { | 147 | public void setHostProviderService(HostProviderService service) { |
149 | this.hostProviderService = service; | 148 | this.hostProviderService = service; |
150 | } | 149 | } |
151 | 150 | ||
152 | - // Return the ProviderId of "this" | 151 | + /* |
152 | + * Return the ProviderId of "this" | ||
153 | + */ | ||
153 | public ProviderId id() { | 154 | public ProviderId id() { |
154 | return providerId; | 155 | return providerId; |
155 | } | 156 | } |
... | @@ -161,7 +162,7 @@ public class HostsWebResource extends AbstractWebResource { | ... | @@ -161,7 +162,7 @@ public class HostsWebResource extends AbstractWebResource { |
161 | */ | 162 | */ |
162 | private HostId parseHost(JsonNode node) { | 163 | private HostId parseHost(JsonNode node) { |
163 | MacAddress mac = MacAddress.valueOf(node.get("mac").asText()); | 164 | MacAddress mac = MacAddress.valueOf(node.get("mac").asText()); |
164 | - VlanId vlanId = VlanId.vlanId(((short) node.get("vlan").asInt((VlanId.UNTAGGED)))); | 165 | + VlanId vlanId = VlanId.vlanId((short) node.get("vlan").asInt(VlanId.UNTAGGED)); |
165 | JsonNode locationNode = node.get("location"); | 166 | JsonNode locationNode = node.get("location"); |
166 | String deviceAndPort = locationNode.get("elementId").asText() + "/" + | 167 | String deviceAndPort = locationNode.get("elementId").asText() + "/" + |
167 | locationNode.get("port").asText(); | 168 | locationNode.get("port").asText(); | ... | ... |
... | @@ -35,7 +35,11 @@ import static org.onosproject.net.PortNumber.portNumber; | ... | @@ -35,7 +35,11 @@ import static org.onosproject.net.PortNumber.portNumber; |
35 | @Path("links") | 35 | @Path("links") |
36 | public class LinksWebResource extends AbstractWebResource { | 36 | public class LinksWebResource extends AbstractWebResource { |
37 | 37 | ||
38 | - enum Direction { ALL, INGRESS, EGRESS } | 38 | + enum Direction { |
39 | + ALL, | ||
40 | + INGRESS, | ||
41 | + EGRESS | ||
42 | + } | ||
39 | 43 | ||
40 | @GET | 44 | @GET |
41 | public Response getLinks(@QueryParam("device") String deviceId, | 45 | public Response getLinks(@QueryParam("device") String deviceId, | ... | ... |
... | @@ -280,13 +280,11 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -280,13 +280,11 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
280 | @SuppressWarnings("unchecked") | 280 | @SuppressWarnings("unchecked") |
281 | public Response delete() { | 281 | public Response delete() { |
282 | NetworkConfigService service = get(NetworkConfigService.class); | 282 | NetworkConfigService service = get(NetworkConfigService.class); |
283 | - service.getSubjectClasses().forEach(subjectClass -> { | 283 | + service.getSubjectClasses() |
284 | - service.getSubjects(subjectClass).forEach(subject -> { | 284 | + .forEach(subjectClass -> service.getSubjects(subjectClass) |
285 | - service.getConfigs(subject).forEach(config -> { | 285 | + .forEach(subject -> service.getConfigs(subject) |
286 | - service.removeConfig(subject, config.getClass()); | 286 | + .forEach(config -> service |
287 | - }); | 287 | + .removeConfig(subject, config.getClass())))); |
288 | - }); | ||
289 | - }); | ||
290 | return Response.ok().build(); | 288 | return Response.ok().build(); |
291 | } | 289 | } |
292 | 290 | ||
... | @@ -303,11 +301,10 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -303,11 +301,10 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
303 | @SuppressWarnings("unchecked") | 301 | @SuppressWarnings("unchecked") |
304 | public Response delete(@PathParam("subjectKey") String subjectKey) { | 302 | public Response delete(@PathParam("subjectKey") String subjectKey) { |
305 | NetworkConfigService service = get(NetworkConfigService.class); | 303 | NetworkConfigService service = get(NetworkConfigService.class); |
306 | - service.getSubjects(service.getSubjectFactory(subjectKey).getClass()).forEach(subject -> { | 304 | + service.getSubjects(service.getSubjectFactory(subjectKey).getClass()) |
307 | - service.getConfigs(subject).forEach(config -> { | 305 | + .forEach(subject -> service.getConfigs(subject) |
308 | - service.removeConfig(subject, config.getClass()); | 306 | + .forEach(config -> service |
309 | - }); | 307 | + .removeConfig(subject, config.getClass()))); |
310 | - }); | ||
311 | return Response.ok().build(); | 308 | return Response.ok().build(); |
312 | } | 309 | } |
313 | } | 310 | } | ... | ... |
... | @@ -38,25 +38,16 @@ import org.onosproject.rest.AbstractWebResource; | ... | @@ -38,25 +38,16 @@ import org.onosproject.rest.AbstractWebResource; |
38 | @Path("paths") | 38 | @Path("paths") |
39 | public class PathsWebResource extends AbstractWebResource { | 39 | public class PathsWebResource extends AbstractWebResource { |
40 | 40 | ||
41 | - // Host id format is 00:00:00:00:00:01/-1 | ||
42 | - private static final int VLAN_SEPARATOR_OFFSET = 17; | ||
43 | - private static final int FIRST_MAC_ADDRESS_SEPARATOR_OFFSET = 2; | ||
44 | - private static final int SECOND_MAC_ADDRESS_SEPARATOR_OFFSET = 5; | ||
45 | - private static final int THIRD_MAC_ADDRESS_SEPARATOR_OFFSET = 8; | ||
46 | - | ||
47 | /** | 41 | /** |
48 | * Determines if the id appears to be the id of a host. | 42 | * Determines if the id appears to be the id of a host. |
43 | + * Host id format is 00:00:00:00:00:01/-1 | ||
49 | * | 44 | * |
50 | * @param id id string | 45 | * @param id id string |
51 | * @return HostId if the id is valid, null otherwise | 46 | * @return HostId if the id is valid, null otherwise |
52 | */ | 47 | */ |
53 | private HostId isHostId(String id) { | 48 | private HostId isHostId(String id) { |
54 | 49 | ||
55 | - if (id.length() < VLAN_SEPARATOR_OFFSET + 1 || | 50 | + if (!id.matches("..:..:..:..:..:../.*")) { |
56 | - id.charAt(FIRST_MAC_ADDRESS_SEPARATOR_OFFSET) != ':' || | ||
57 | - id.charAt(SECOND_MAC_ADDRESS_SEPARATOR_OFFSET) != ':' || | ||
58 | - id.charAt(THIRD_MAC_ADDRESS_SEPARATOR_OFFSET) != ':' || | ||
59 | - id.charAt(VLAN_SEPARATOR_OFFSET) != '/') { | ||
60 | return null; | 51 | return null; |
61 | } | 52 | } |
62 | 53 | ... | ... |
-
Please register or login to post a comment