Committed by
Gerrit Code Review
Added more network config validation checks
Change-Id: I9e810206fc3e744c86f854865dfdd9614a84fdf7
Showing
2 changed files
with
78 additions
and
4 deletions
| ... | @@ -26,6 +26,7 @@ import com.google.common.collect.Lists; | ... | @@ -26,6 +26,7 @@ import com.google.common.collect.Lists; |
| 26 | import org.onlab.packet.IpAddress; | 26 | import org.onlab.packet.IpAddress; |
| 27 | import org.onlab.packet.IpPrefix; | 27 | import org.onlab.packet.IpPrefix; |
| 28 | import org.onlab.packet.MacAddress; | 28 | import org.onlab.packet.MacAddress; |
| 29 | +import org.onlab.packet.TpPort; | ||
| 29 | import org.onosproject.net.ConnectPoint; | 30 | import org.onosproject.net.ConnectPoint; |
| 30 | 31 | ||
| 31 | import java.util.Collection; | 32 | import java.util.Collection; |
| ... | @@ -397,7 +398,7 @@ public abstract class Config<S> { | ... | @@ -397,7 +398,7 @@ public abstract class Config<S> { |
| 397 | * Indicates whether only the specified fields are present in the backing JSON. | 398 | * Indicates whether only the specified fields are present in the backing JSON. |
| 398 | * | 399 | * |
| 399 | * @param allowedFields allowed field names | 400 | * @param allowedFields allowed field names |
| 400 | - * @return true if all allowedFields are present; false otherwise | 401 | + * @return true if only allowedFields are present; false otherwise |
| 401 | */ | 402 | */ |
| 402 | protected boolean hasOnlyFields(String... allowedFields) { | 403 | protected boolean hasOnlyFields(String... allowedFields) { |
| 403 | return hasOnlyFields(object, allowedFields); | 404 | return hasOnlyFields(object, allowedFields); |
| ... | @@ -409,7 +410,7 @@ public abstract class Config<S> { | ... | @@ -409,7 +410,7 @@ public abstract class Config<S> { |
| 409 | * | 410 | * |
| 410 | * @param node node whose fields to check | 411 | * @param node node whose fields to check |
| 411 | * @param allowedFields allowed field names | 412 | * @param allowedFields allowed field names |
| 412 | - * @return true if all allowedFields are present; false otherwise | 413 | + * @return true if only allowedFields are present; false otherwise |
| 413 | */ | 414 | */ |
| 414 | protected boolean hasOnlyFields(ObjectNode node, String... allowedFields) { | 415 | protected boolean hasOnlyFields(ObjectNode node, String... allowedFields) { |
| 415 | Set<String> fields = ImmutableSet.copyOf(allowedFields); | 416 | Set<String> fields = ImmutableSet.copyOf(allowedFields); |
| ... | @@ -417,6 +418,29 @@ public abstract class Config<S> { | ... | @@ -417,6 +418,29 @@ public abstract class Config<S> { |
| 417 | } | 418 | } |
| 418 | 419 | ||
| 419 | /** | 420 | /** |
| 421 | + * Indicates whether all specified fields are present in the backing JSON. | ||
| 422 | + * | ||
| 423 | + * @param mandatoryFields mandatory field names | ||
| 424 | + * @return true if all mandatory fields are present; false otherwise | ||
| 425 | + */ | ||
| 426 | + protected boolean hasFields(String... mandatoryFields) { | ||
| 427 | + return hasFields(object, mandatoryFields); | ||
| 428 | + } | ||
| 429 | + | ||
| 430 | + /** | ||
| 431 | + * Indicates whether all specified fields are present in a particular | ||
| 432 | + * JSON object. | ||
| 433 | + * | ||
| 434 | + * @param node node whose fields to check | ||
| 435 | + * @param mandatoryFields mandatory field names | ||
| 436 | + * @return true if all mandatory fields are present; false otherwise | ||
| 437 | + */ | ||
| 438 | + protected boolean hasFields(ObjectNode node, String... mandatoryFields) { | ||
| 439 | + Set<String> fields = ImmutableSet.copyOf(mandatoryFields); | ||
| 440 | + return Iterators.all(fields.iterator(), f -> !node.path(f).isMissingNode()); | ||
| 441 | + } | ||
| 442 | + | ||
| 443 | + /** | ||
| 420 | * Indicates whether the specified field holds a valid MAC address. | 444 | * Indicates whether the specified field holds a valid MAC address. |
| 421 | * | 445 | * |
| 422 | * @param field JSON field name | 446 | * @param field JSON field name |
| ... | @@ -505,6 +529,34 @@ public abstract class Config<S> { | ... | @@ -505,6 +529,34 @@ public abstract class Config<S> { |
| 505 | } | 529 | } |
| 506 | 530 | ||
| 507 | /** | 531 | /** |
| 532 | + * Indicates whether the specified field holds a valid transport layer port. | ||
| 533 | + * | ||
| 534 | + * @param field JSON field name | ||
| 535 | + * @param presence specifies if field is optional or mandatory | ||
| 536 | + * @return true if valid; false otherwise | ||
| 537 | + * @throws IllegalArgumentException if field is present, but not valid value | ||
| 538 | + */ | ||
| 539 | + protected boolean isTpPort(String field, FieldPresence presence) { | ||
| 540 | + return isTpPort(object, field, presence); | ||
| 541 | + } | ||
| 542 | + | ||
| 543 | + /** | ||
| 544 | + * Indicates whether the specified field of a particular node holds a valid | ||
| 545 | + * transport layer port. | ||
| 546 | + * | ||
| 547 | + * @param objectNode node from whom to access the field | ||
| 548 | + * @param field JSON field name | ||
| 549 | + * @param presence specifies if field is optional or mandatory | ||
| 550 | + * @return true if valid; false otherwise | ||
| 551 | + * @throws IllegalArgumentException if field is present, but not valid value | ||
| 552 | + */ | ||
| 553 | + protected boolean isTpPort(ObjectNode objectNode, String field, FieldPresence presence) { | ||
| 554 | + JsonNode node = objectNode.path(field); | ||
| 555 | + return isValid(node, presence, node.isNumber() && | ||
| 556 | + TpPort.tpPort(node.asInt()) != null); | ||
| 557 | + } | ||
| 558 | + | ||
| 559 | + /** | ||
| 508 | * Indicates whether the specified field holds a valid connect point string. | 560 | * Indicates whether the specified field holds a valid connect point string. |
| 509 | * | 561 | * |
| 510 | * @param field JSON field name | 562 | * @param field JSON field name | ... | ... |
| ... | @@ -39,6 +39,8 @@ public class ConfigTest { | ... | @@ -39,6 +39,8 @@ public class ConfigTest { |
| 39 | private static final String DOUBLE = "double"; | 39 | private static final String DOUBLE = "double"; |
| 40 | private static final String MAC = "mac"; | 40 | private static final String MAC = "mac"; |
| 41 | private static final String IP = "ip"; | 41 | private static final String IP = "ip"; |
| 42 | + private static final String TP_PORT = "tpPort"; | ||
| 43 | + private static final String BAD_TP_PORT = "badTpPort"; | ||
| 42 | 44 | ||
| 43 | private final ObjectMapper mapper = new ObjectMapper(); | 45 | private final ObjectMapper mapper = new ObjectMapper(); |
| 44 | private final ConfigApplyDelegate delegate = new TestDelegate(); | 46 | private final ConfigApplyDelegate delegate = new TestDelegate(); |
| ... | @@ -50,19 +52,27 @@ public class ConfigTest { | ... | @@ -50,19 +52,27 @@ public class ConfigTest { |
| 50 | public void setUp() { | 52 | public void setUp() { |
| 51 | json = new ObjectMapper().createObjectNode() | 53 | json = new ObjectMapper().createObjectNode() |
| 52 | .put(TEXT, "foo").put(LONG, 5).put(DOUBLE, 0.5) | 54 | .put(TEXT, "foo").put(LONG, 5).put(DOUBLE, 0.5) |
| 53 | - .put(MAC, "ab:cd:ef:ca:fe:ed").put(IP, "12.34.56.78"); | 55 | + .put(MAC, "ab:cd:ef:ca:fe:ed").put(IP, "12.34.56.78") |
| 56 | + .put(TP_PORT, 65535).put(BAD_TP_PORT, 65536); | ||
| 54 | cfg = new TestConfig(); | 57 | cfg = new TestConfig(); |
| 55 | cfg.init(SUBJECT, KEY, json, mapper, delegate); | 58 | cfg.init(SUBJECT, KEY, json, mapper, delegate); |
| 56 | } | 59 | } |
| 57 | 60 | ||
| 58 | @Test | 61 | @Test |
| 59 | public void hasOnlyFields() { | 62 | public void hasOnlyFields() { |
| 60 | - assertTrue("has unexpected fields", cfg.hasOnlyFields(TEXT, LONG, DOUBLE, MAC, IP)); | 63 | + assertTrue("has unexpected fields", cfg.hasOnlyFields(TEXT, LONG, DOUBLE, MAC, IP, |
| 64 | + TP_PORT, BAD_TP_PORT)); | ||
| 61 | assertFalse("did not detect unexpected fields", cfg.hasOnlyFields(TEXT, LONG, DOUBLE, MAC)); | 65 | assertFalse("did not detect unexpected fields", cfg.hasOnlyFields(TEXT, LONG, DOUBLE, MAC)); |
| 62 | assertTrue("is not proper text", cfg.isString(TEXT, MANDATORY)); | 66 | assertTrue("is not proper text", cfg.isString(TEXT, MANDATORY)); |
| 63 | } | 67 | } |
| 64 | 68 | ||
| 65 | @Test | 69 | @Test |
| 70 | + public void hasFields() { | ||
| 71 | + assertTrue("does not have mandatory field", cfg.hasFields(TEXT, LONG, DOUBLE, MAC)); | ||
| 72 | + assertFalse("did not detect missing field", cfg.hasFields("none")); | ||
| 73 | + } | ||
| 74 | + | ||
| 75 | + @Test | ||
| 66 | public void isString() { | 76 | public void isString() { |
| 67 | assertTrue("is not proper text", cfg.isString(TEXT, MANDATORY)); | 77 | assertTrue("is not proper text", cfg.isString(TEXT, MANDATORY)); |
| 68 | assertTrue("is not proper text", cfg.isString(TEXT, MANDATORY, "^f.*")); | 78 | assertTrue("is not proper text", cfg.isString(TEXT, MANDATORY, "^f.*")); |
| ... | @@ -127,6 +137,18 @@ public class ConfigTest { | ... | @@ -127,6 +137,18 @@ public class ConfigTest { |
| 127 | assertTrue("is not proper ip", cfg.isIpAddress(TEXT, MANDATORY)); | 137 | assertTrue("is not proper ip", cfg.isIpAddress(TEXT, MANDATORY)); |
| 128 | } | 138 | } |
| 129 | 139 | ||
| 140 | + @Test | ||
| 141 | + public void isTpPort() { | ||
| 142 | + assertTrue("is not proper transport port", cfg.isTpPort(TP_PORT, MANDATORY)); | ||
| 143 | + assertTrue("is not proper transport port", cfg.isTpPort(TP_PORT, OPTIONAL)); | ||
| 144 | + assertTrue("is not proper transport port", cfg.isTpPort("none", OPTIONAL)); | ||
| 145 | + assertFalse("did not detect missing field", cfg.isTpPort("none", MANDATORY)); | ||
| 146 | + } | ||
| 147 | + | ||
| 148 | + @Test(expected = IllegalArgumentException.class) | ||
| 149 | + public void badTpPort() { | ||
| 150 | + assertTrue("is not proper transport port", cfg.isTpPort(BAD_TP_PORT, MANDATORY)); | ||
| 151 | + } | ||
| 130 | 152 | ||
| 131 | // TODO: Add tests for other helper methods | 153 | // TODO: Add tests for other helper methods |
| 132 | 154 | ... | ... |
-
Please register or login to post a comment