Committed by
Gerrit Code Review
Allow list-type config to be POSTed to subjectkey/subject/configkey endpoint.
Also add validation that the given JSON node is appropriate for the config type (list vs object). Change-Id: Ib1c12b538860a6f18b8311c5f5a786608c04beb8
Showing
2 changed files
with
62 additions
and
35 deletions
| ... | @@ -61,7 +61,11 @@ import java.util.Objects; | ... | @@ -61,7 +61,11 @@ import java.util.Objects; |
| 61 | import java.util.Set; | 61 | import java.util.Set; |
| 62 | 62 | ||
| 63 | import static com.google.common.base.Preconditions.checkArgument; | 63 | import static com.google.common.base.Preconditions.checkArgument; |
| 64 | -import static org.onosproject.net.config.NetworkConfigEvent.Type.*; | 64 | +import static org.onosproject.net.config.NetworkConfigEvent.Type.CONFIG_ADDED; |
| 65 | +import static org.onosproject.net.config.NetworkConfigEvent.Type.CONFIG_REGISTERED; | ||
| 66 | +import static org.onosproject.net.config.NetworkConfigEvent.Type.CONFIG_REMOVED; | ||
| 67 | +import static org.onosproject.net.config.NetworkConfigEvent.Type.CONFIG_UNREGISTERED; | ||
| 68 | +import static org.onosproject.net.config.NetworkConfigEvent.Type.CONFIG_UPDATED; | ||
| 65 | 69 | ||
| 66 | /** | 70 | /** |
| 67 | * Implementation of a distributed network configuration store. | 71 | * Implementation of a distributed network configuration store. |
| ... | @@ -77,6 +81,10 @@ public class DistributedNetworkConfigStore | ... | @@ -77,6 +81,10 @@ public class DistributedNetworkConfigStore |
| 77 | private static final int MAX_BACKOFF = 10; | 81 | private static final int MAX_BACKOFF = 10; |
| 78 | private static final String INVALID_CONFIG_JSON = | 82 | private static final String INVALID_CONFIG_JSON = |
| 79 | "JSON node does not contain valid configuration"; | 83 | "JSON node does not contain valid configuration"; |
| 84 | + private static final String INVALID_JSON_LIST = | ||
| 85 | + "JSON node is not a list for list type config"; | ||
| 86 | + private static final String INVALID_JSON_OBJECT = | ||
| 87 | + "JSON node is not an object for object type config"; | ||
| 80 | 88 | ||
| 81 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) | 89 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) |
| 82 | protected StorageService storageService; | 90 | protected StorageService storageService; |
| ... | @@ -262,47 +270,67 @@ public class DistributedNetworkConfigStore | ... | @@ -262,47 +270,67 @@ public class DistributedNetworkConfigStore |
| 262 | * @return config object or null of no factory found or if the specified | 270 | * @return config object or null of no factory found or if the specified |
| 263 | * JSON is null | 271 | * JSON is null |
| 264 | */ | 272 | */ |
| 265 | - @SuppressWarnings("unchecked") | ||
| 266 | private <S, C extends Config<S>> C createConfig(S subject, Class<C> configClass, | 273 | private <S, C extends Config<S>> C createConfig(S subject, Class<C> configClass, |
| 267 | JsonNode json) { | 274 | JsonNode json) { |
| 268 | - if (json != null) { | 275 | + return createConfig(subject, configClass, json, false); |
| 269 | - ConfigFactory<S, C> factory = factoriesByConfig.get(configClass.getName()); | ||
| 270 | - if (factory != null) { | ||
| 271 | - C config = factory.createConfig(); | ||
| 272 | - config.init(subject, factory.configKey(), json, mapper, applyDelegate); | ||
| 273 | - return config; | ||
| 274 | - } | ||
| 275 | - } | ||
| 276 | - return null; | ||
| 277 | } | 276 | } |
| 278 | 277 | ||
| 279 | /** | 278 | /** |
| 280 | - * Produces a detached config from the specified subject, config class and | 279 | + * Produces a config from the specified subject, config class and raw JSON. |
| 281 | - * raw JSON. | ||
| 282 | * | 280 | * |
| 283 | - * A detached config can no longer be applied. This should be used only for | 281 | + * The config can optionally be detached, which means it does not contain a |
| 284 | - * passing the config object in the NetworkConfigEvent. | 282 | + * reference to an apply delegate. This means a detached config can not be |
| 283 | + * applied. This should be used only for passing the config object in the | ||
| 284 | + * NetworkConfigEvent. | ||
| 285 | * | 285 | * |
| 286 | * @param subject config subject | 286 | * @param subject config subject |
| 287 | * @param configClass config class | 287 | * @param configClass config class |
| 288 | * @param json raw JSON data | 288 | * @param json raw JSON data |
| 289 | + * @param detached whether the config should be detached, that is, should | ||
| 290 | + * be created without setting an apply delegate. | ||
| 289 | * @return config object or null of no factory found or if the specified | 291 | * @return config object or null of no factory found or if the specified |
| 290 | * JSON is null | 292 | * JSON is null |
| 291 | */ | 293 | */ |
| 292 | @SuppressWarnings("unchecked") | 294 | @SuppressWarnings("unchecked") |
| 293 | - private Config createDetachedConfig(Object subject, | 295 | + private <S, C extends Config<S>> C createConfig(S subject, Class<C> configClass, |
| 294 | - Class configClass, JsonNode json) { | 296 | + JsonNode json, boolean detached) { |
| 295 | if (json != null) { | 297 | if (json != null) { |
| 296 | - ConfigFactory factory = factoriesByConfig.get(configClass.getName()); | 298 | + ConfigFactory<S, C> factory = factoriesByConfig.get(configClass.getName()); |
| 297 | if (factory != null) { | 299 | if (factory != null) { |
| 298 | - Config config = factory.createConfig(); | 300 | + validateJsonType(json, factory); |
| 299 | - config.init(subject, factory.configKey(), json, mapper, null); | 301 | + C config = factory.createConfig(); |
| 302 | + config.init(subject, factory.configKey(), json, mapper, | ||
| 303 | + detached ? null : applyDelegate); | ||
| 300 | return config; | 304 | return config; |
| 301 | } | 305 | } |
| 302 | } | 306 | } |
| 303 | return null; | 307 | return null; |
| 304 | } | 308 | } |
| 305 | 309 | ||
| 310 | + /** | ||
| 311 | + * Validates that the type of the JSON node is appropriate for the type of | ||
| 312 | + * configuration. A list type configuration must be created with an | ||
| 313 | + * ArrayNode, and an object type configuration must be created with an | ||
| 314 | + * ObjectNode. | ||
| 315 | + * | ||
| 316 | + * @param json JSON node to check | ||
| 317 | + * @param factory config factory of configuration | ||
| 318 | + * @param <S> subject | ||
| 319 | + * @param <C> configuration | ||
| 320 | + * @return true if the JSON node type is appropriate for the configuration | ||
| 321 | + */ | ||
| 322 | + private <S, C extends Config<S>> boolean validateJsonType(JsonNode json, | ||
| 323 | + ConfigFactory<S, C> factory) { | ||
| 324 | + if (factory.isList() && !(json instanceof ArrayNode)) { | ||
| 325 | + throw new IllegalArgumentException(INVALID_JSON_LIST); | ||
| 326 | + } | ||
| 327 | + if (!factory.isList() && !(json instanceof ObjectNode)) { | ||
| 328 | + throw new IllegalArgumentException(INVALID_JSON_OBJECT); | ||
| 329 | + } | ||
| 330 | + | ||
| 331 | + return true; | ||
| 332 | + } | ||
| 333 | + | ||
| 306 | 334 | ||
| 307 | // Auxiliary delegate to receive notifications about changes applied to | 335 | // Auxiliary delegate to receive notifications about changes applied to |
| 308 | // the network configuration - by the apps. | 336 | // the network configuration - by the apps. |
| ... | @@ -380,11 +408,11 @@ public class DistributedNetworkConfigStore | ... | @@ -380,11 +408,11 @@ public class DistributedNetworkConfigStore |
| 380 | Versioned<JsonNode> oldValue = event.oldValue(); | 408 | Versioned<JsonNode> oldValue = event.oldValue(); |
| 381 | 409 | ||
| 382 | Config config = (newValue != null) ? | 410 | Config config = (newValue != null) ? |
| 383 | - createDetachedConfig(subject, configClass, newValue.value()) : | 411 | + createConfig(subject, configClass, newValue.value(), true) : |
| 384 | - null; | 412 | + null; |
| 385 | Config prevConfig = (oldValue != null) ? | 413 | Config prevConfig = (oldValue != null) ? |
| 386 | - createDetachedConfig(subject, configClass, oldValue.value()) : | 414 | + createConfig(subject, configClass, oldValue.value(), true) : |
| 387 | - null; | 415 | + null; |
| 388 | 416 | ||
| 389 | NetworkConfigEvent.Type type; | 417 | NetworkConfigEvent.Type type; |
| 390 | switch (event.type()) { | 418 | switch (event.type()) { | ... | ... |
| ... | @@ -15,9 +15,12 @@ | ... | @@ -15,9 +15,12 @@ |
| 15 | */ | 15 | */ |
| 16 | package org.onosproject.rest.resources; | 16 | package org.onosproject.rest.resources; |
| 17 | 17 | ||
| 18 | -import java.io.IOException; | 18 | +import com.fasterxml.jackson.databind.JsonNode; |
| 19 | -import java.io.InputStream; | 19 | +import com.fasterxml.jackson.databind.node.ObjectNode; |
| 20 | -import java.util.Set; | 20 | +import org.onosproject.net.config.Config; |
| 21 | +import org.onosproject.net.config.NetworkConfigService; | ||
| 22 | +import org.onosproject.net.config.SubjectFactory; | ||
| 23 | +import org.onosproject.rest.AbstractWebResource; | ||
| 21 | 24 | ||
| 22 | import javax.ws.rs.Consumes; | 25 | import javax.ws.rs.Consumes; |
| 23 | import javax.ws.rs.DELETE; | 26 | import javax.ws.rs.DELETE; |
| ... | @@ -28,13 +31,9 @@ import javax.ws.rs.PathParam; | ... | @@ -28,13 +31,9 @@ import javax.ws.rs.PathParam; |
| 28 | import javax.ws.rs.Produces; | 31 | import javax.ws.rs.Produces; |
| 29 | import javax.ws.rs.core.MediaType; | 32 | import javax.ws.rs.core.MediaType; |
| 30 | import javax.ws.rs.core.Response; | 33 | import javax.ws.rs.core.Response; |
| 31 | - | 34 | +import java.io.IOException; |
| 32 | -import org.onosproject.net.config.Config; | 35 | +import java.io.InputStream; |
| 33 | -import org.onosproject.net.config.NetworkConfigService; | 36 | +import java.util.Set; |
| 34 | -import org.onosproject.net.config.SubjectFactory; | ||
| 35 | -import org.onosproject.rest.AbstractWebResource; | ||
| 36 | - | ||
| 37 | -import com.fasterxml.jackson.databind.node.ObjectNode; | ||
| 38 | 37 | ||
| 39 | import static org.onlab.util.Tools.emptyIsNotFound; | 38 | import static org.onlab.util.Tools.emptyIsNotFound; |
| 40 | import static org.onlab.util.Tools.nullIsNotFound; | 39 | import static org.onlab.util.Tools.nullIsNotFound; |
| ... | @@ -263,7 +262,7 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -263,7 +262,7 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
| 263 | @PathParam("configKey") String configKey, | 262 | @PathParam("configKey") String configKey, |
| 264 | InputStream request) throws IOException { | 263 | InputStream request) throws IOException { |
| 265 | NetworkConfigService service = get(NetworkConfigService.class); | 264 | NetworkConfigService service = get(NetworkConfigService.class); |
| 266 | - ObjectNode root = (ObjectNode) mapper().readTree(request); | 265 | + JsonNode root = mapper().readTree(request); |
| 267 | service.applyConfig(subjectClassKey, | 266 | service.applyConfig(subjectClassKey, |
| 268 | service.getSubjectFactory(subjectClassKey).createSubject(subjectKey), | 267 | service.getSubjectFactory(subjectClassKey).createSubject(subjectKey), |
| 269 | configKey, root); | 268 | configKey, root); | ... | ... |
-
Please register or login to post a comment