Committed by
Gerrit Code Review
Improve network config validation errors to show which fields are invalid.
Previously, uploading invalid config results in a generic error message which makes it difficult to figure out what is wrong with the config Change-Id: I307d2fc0669679b067389c722556eef3aae098b9
Showing
8 changed files
with
537 additions
and
126 deletions
| ... | @@ -21,7 +21,6 @@ import com.fasterxml.jackson.databind.node.ArrayNode; | ... | @@ -21,7 +21,6 @@ import com.fasterxml.jackson.databind.node.ArrayNode; |
| 21 | import com.fasterxml.jackson.databind.node.ObjectNode; | 21 | import com.fasterxml.jackson.databind.node.ObjectNode; |
| 22 | import com.google.common.annotations.Beta; | 22 | import com.google.common.annotations.Beta; |
| 23 | import com.google.common.collect.ImmutableSet; | 23 | import com.google.common.collect.ImmutableSet; |
| 24 | -import com.google.common.collect.Iterators; | ||
| 25 | import com.google.common.collect.Lists; | 24 | import com.google.common.collect.Lists; |
| 26 | import org.onlab.packet.IpAddress; | 25 | import org.onlab.packet.IpAddress; |
| 27 | import org.onlab.packet.IpPrefix; | 26 | import org.onlab.packet.IpPrefix; |
| ... | @@ -103,7 +102,7 @@ public abstract class Config<S> { | ... | @@ -103,7 +102,7 @@ public abstract class Config<S> { |
| 103 | * Default implementation returns true. | 102 | * Default implementation returns true. |
| 104 | * Subclasses are expected to override this with their own validation. | 103 | * Subclasses are expected to override this with their own validation. |
| 105 | * Implementations are free to throw a RuntimeException if data is invalid. | 104 | * Implementations are free to throw a RuntimeException if data is invalid. |
| 106 | - * * </p> | 105 | + * </p> |
| 107 | * | 106 | * |
| 108 | * @return true if the data is valid; false otherwise | 107 | * @return true if the data is valid; false otherwise |
| 109 | * @throws RuntimeException if configuration is invalid or completely foobar | 108 | * @throws RuntimeException if configuration is invalid or completely foobar |
| ... | @@ -114,11 +113,13 @@ public abstract class Config<S> { | ... | @@ -114,11 +113,13 @@ public abstract class Config<S> { |
| 114 | // isString(path) | 113 | // isString(path) |
| 115 | // isBoolean(path) | 114 | // isBoolean(path) |
| 116 | // isNumber(path, [min, max]) | 115 | // isNumber(path, [min, max]) |
| 116 | + // isIntegralNumber(path, [min, max]) | ||
| 117 | // isDecimal(path, [min, max]) | 117 | // isDecimal(path, [min, max]) |
| 118 | // isMacAddress(path) | 118 | // isMacAddress(path) |
| 119 | // isIpAddress(path) | 119 | // isIpAddress(path) |
| 120 | // isIpPrefix(path) | 120 | // isIpPrefix(path) |
| 121 | // isConnectPoint(path) | 121 | // isConnectPoint(path) |
| 122 | + // isTpPort(path) | ||
| 122 | return true; | 123 | return true; |
| 123 | } | 124 | } |
| 124 | 125 | ||
| ... | @@ -153,8 +154,9 @@ public abstract class Config<S> { | ... | @@ -153,8 +154,9 @@ public abstract class Config<S> { |
| 153 | 154 | ||
| 154 | /** | 155 | /** |
| 155 | * Applies any configuration changes made via this configuration. | 156 | * Applies any configuration changes made via this configuration. |
| 156 | - * | 157 | + * <p> |
| 157 | * Not effective for detached configs. | 158 | * Not effective for detached configs. |
| 159 | + * </p> | ||
| 158 | */ | 160 | */ |
| 159 | public void apply() { | 161 | public void apply() { |
| 160 | checkState(delegate != null, "Cannot apply detached config"); | 162 | checkState(delegate != null, "Cannot apply detached config"); |
| ... | @@ -414,7 +416,12 @@ public abstract class Config<S> { | ... | @@ -414,7 +416,12 @@ public abstract class Config<S> { |
| 414 | */ | 416 | */ |
| 415 | protected boolean hasOnlyFields(ObjectNode node, String... allowedFields) { | 417 | protected boolean hasOnlyFields(ObjectNode node, String... allowedFields) { |
| 416 | Set<String> fields = ImmutableSet.copyOf(allowedFields); | 418 | Set<String> fields = ImmutableSet.copyOf(allowedFields); |
| 417 | - return !Iterators.any(node.fieldNames(), f -> !fields.contains(f)); | 419 | + node.fieldNames().forEachRemaining(f -> { |
| 420 | + if (!fields.contains(f)) { | ||
| 421 | + throw new InvalidFieldException(f, "Field is not allowed"); | ||
| 422 | + } | ||
| 423 | + }); | ||
| 424 | + return true; | ||
| 418 | } | 425 | } |
| 419 | 426 | ||
| 420 | /** | 427 | /** |
| ... | @@ -437,7 +444,12 @@ public abstract class Config<S> { | ... | @@ -437,7 +444,12 @@ public abstract class Config<S> { |
| 437 | */ | 444 | */ |
| 438 | protected boolean hasFields(ObjectNode node, String... mandatoryFields) { | 445 | protected boolean hasFields(ObjectNode node, String... mandatoryFields) { |
| 439 | Set<String> fields = ImmutableSet.copyOf(mandatoryFields); | 446 | Set<String> fields = ImmutableSet.copyOf(mandatoryFields); |
| 440 | - return Iterators.all(fields.iterator(), f -> !node.path(f).isMissingNode()); | 447 | + fields.forEach(f -> { |
| 448 | + if (node.path(f).isMissingNode()) { | ||
| 449 | + throw new InvalidFieldException(f, "Mandatory field is not present"); | ||
| 450 | + } | ||
| 451 | + }); | ||
| 452 | + return true; | ||
| 441 | } | 453 | } |
| 442 | 454 | ||
| 443 | /** | 455 | /** |
| ... | @@ -446,12 +458,10 @@ public abstract class Config<S> { | ... | @@ -446,12 +458,10 @@ public abstract class Config<S> { |
| 446 | * @param field JSON field name | 458 | * @param field JSON field name |
| 447 | * @param presence specifies if field is optional or mandatory | 459 | * @param presence specifies if field is optional or mandatory |
| 448 | * @return true if valid; false otherwise | 460 | * @return true if valid; false otherwise |
| 449 | - * @throws IllegalArgumentException if field is present, but not valid MAC | 461 | + * @throws InvalidFieldException if the field is present but not valid |
| 450 | */ | 462 | */ |
| 451 | protected boolean isMacAddress(String field, FieldPresence presence) { | 463 | protected boolean isMacAddress(String field, FieldPresence presence) { |
| 452 | - JsonNode node = object.path(field); | 464 | + return isMacAddress(object, field, presence); |
| 453 | - return isValid(node, presence, node.isTextual() && | ||
| 454 | - MacAddress.valueOf(node.asText()) != null); | ||
| 455 | } | 465 | } |
| 456 | 466 | ||
| 457 | /** | 467 | /** |
| ... | @@ -462,12 +472,13 @@ public abstract class Config<S> { | ... | @@ -462,12 +472,13 @@ public abstract class Config<S> { |
| 462 | * @param field JSON field name | 472 | * @param field JSON field name |
| 463 | * @param presence specifies if field is optional or mandatory | 473 | * @param presence specifies if field is optional or mandatory |
| 464 | * @return true if valid; false otherwise | 474 | * @return true if valid; false otherwise |
| 465 | - * @throws IllegalArgumentException if field is present, but not valid MAC | 475 | + * @throws InvalidFieldException if the field is present but not valid |
| 466 | */ | 476 | */ |
| 467 | protected boolean isMacAddress(ObjectNode objectNode, String field, FieldPresence presence) { | 477 | protected boolean isMacAddress(ObjectNode objectNode, String field, FieldPresence presence) { |
| 468 | - JsonNode node = objectNode.path(field); | 478 | + return isValid(objectNode, field, presence, n -> { |
| 469 | - return isValid(node, presence, node.isTextual() && | 479 | + MacAddress.valueOf(n.asText()); |
| 470 | - MacAddress.valueOf(node.asText()) != null); | 480 | + return true; |
| 481 | + }); | ||
| 471 | } | 482 | } |
| 472 | 483 | ||
| 473 | /** | 484 | /** |
| ... | @@ -476,7 +487,7 @@ public abstract class Config<S> { | ... | @@ -476,7 +487,7 @@ public abstract class Config<S> { |
| 476 | * @param field JSON field name | 487 | * @param field JSON field name |
| 477 | * @param presence specifies if field is optional or mandatory | 488 | * @param presence specifies if field is optional or mandatory |
| 478 | * @return true if valid; false otherwise | 489 | * @return true if valid; false otherwise |
| 479 | - * @throws IllegalArgumentException if field is present, but not valid IP | 490 | + * @throws InvalidFieldException if the field is present but not valid |
| 480 | */ | 491 | */ |
| 481 | protected boolean isIpAddress(String field, FieldPresence presence) { | 492 | protected boolean isIpAddress(String field, FieldPresence presence) { |
| 482 | return isIpAddress(object, field, presence); | 493 | return isIpAddress(object, field, presence); |
| ... | @@ -490,12 +501,13 @@ public abstract class Config<S> { | ... | @@ -490,12 +501,13 @@ public abstract class Config<S> { |
| 490 | * @param field JSON field name | 501 | * @param field JSON field name |
| 491 | * @param presence specifies if field is optional or mandatory | 502 | * @param presence specifies if field is optional or mandatory |
| 492 | * @return true if valid; false otherwise | 503 | * @return true if valid; false otherwise |
| 493 | - * @throws IllegalArgumentException if field is present, but not valid IP | 504 | + * @throws InvalidFieldException if the field is present but not valid |
| 494 | */ | 505 | */ |
| 495 | protected boolean isIpAddress(ObjectNode objectNode, String field, FieldPresence presence) { | 506 | protected boolean isIpAddress(ObjectNode objectNode, String field, FieldPresence presence) { |
| 496 | - JsonNode node = objectNode.path(field); | 507 | + return isValid(objectNode, field, presence, n -> { |
| 497 | - return isValid(node, presence, node.isTextual() && | 508 | + IpAddress.valueOf(n.asText()); |
| 498 | - IpAddress.valueOf(node.asText()) != null); | 509 | + return true; |
| 510 | + }); | ||
| 499 | } | 511 | } |
| 500 | 512 | ||
| 501 | /** | 513 | /** |
| ... | @@ -504,8 +516,7 @@ public abstract class Config<S> { | ... | @@ -504,8 +516,7 @@ public abstract class Config<S> { |
| 504 | * @param field JSON field name | 516 | * @param field JSON field name |
| 505 | * @param presence specifies if field is optional or mandatory | 517 | * @param presence specifies if field is optional or mandatory |
| 506 | * @return true if valid; false otherwise | 518 | * @return true if valid; false otherwise |
| 507 | - * @throws IllegalArgumentException if field is present, but not valid IP | 519 | + * @throws InvalidFieldException if the field is present but not valid |
| 508 | - * prefix | ||
| 509 | */ | 520 | */ |
| 510 | protected boolean isIpPrefix(String field, FieldPresence presence) { | 521 | protected boolean isIpPrefix(String field, FieldPresence presence) { |
| 511 | return isIpPrefix(object, field, presence); | 522 | return isIpPrefix(object, field, presence); |
| ... | @@ -519,13 +530,13 @@ public abstract class Config<S> { | ... | @@ -519,13 +530,13 @@ public abstract class Config<S> { |
| 519 | * @param field JSON field name | 530 | * @param field JSON field name |
| 520 | * @param presence specifies if field is optional or mandatory | 531 | * @param presence specifies if field is optional or mandatory |
| 521 | * @return true if valid; false otherwise | 532 | * @return true if valid; false otherwise |
| 522 | - * @throws IllegalArgumentException if field is present, but not valid IP | 533 | + * @throws InvalidFieldException if the field is present but not valid |
| 523 | - * prefix | ||
| 524 | */ | 534 | */ |
| 525 | protected boolean isIpPrefix(ObjectNode objectNode, String field, FieldPresence presence) { | 535 | protected boolean isIpPrefix(ObjectNode objectNode, String field, FieldPresence presence) { |
| 526 | - JsonNode node = objectNode.path(field); | 536 | + return isValid(objectNode, field, presence, n -> { |
| 527 | - return isValid(node, presence, node.isTextual() && | 537 | + IpPrefix.valueOf(n.asText()); |
| 528 | - IpPrefix.valueOf(node.asText()) != null); | 538 | + return true; |
| 539 | + }); | ||
| 529 | } | 540 | } |
| 530 | 541 | ||
| 531 | /** | 542 | /** |
| ... | @@ -534,7 +545,7 @@ public abstract class Config<S> { | ... | @@ -534,7 +545,7 @@ public abstract class Config<S> { |
| 534 | * @param field JSON field name | 545 | * @param field JSON field name |
| 535 | * @param presence specifies if field is optional or mandatory | 546 | * @param presence specifies if field is optional or mandatory |
| 536 | * @return true if valid; false otherwise | 547 | * @return true if valid; false otherwise |
| 537 | - * @throws IllegalArgumentException if field is present, but not valid value | 548 | + * @throws InvalidFieldException if the field is present but not valid |
| 538 | */ | 549 | */ |
| 539 | protected boolean isTpPort(String field, FieldPresence presence) { | 550 | protected boolean isTpPort(String field, FieldPresence presence) { |
| 540 | return isTpPort(object, field, presence); | 551 | return isTpPort(object, field, presence); |
| ... | @@ -548,12 +559,13 @@ public abstract class Config<S> { | ... | @@ -548,12 +559,13 @@ public abstract class Config<S> { |
| 548 | * @param field JSON field name | 559 | * @param field JSON field name |
| 549 | * @param presence specifies if field is optional or mandatory | 560 | * @param presence specifies if field is optional or mandatory |
| 550 | * @return true if valid; false otherwise | 561 | * @return true if valid; false otherwise |
| 551 | - * @throws IllegalArgumentException if field is present, but not valid value | 562 | + * @throws InvalidFieldException if the field is present but not valid |
| 552 | */ | 563 | */ |
| 553 | protected boolean isTpPort(ObjectNode objectNode, String field, FieldPresence presence) { | 564 | protected boolean isTpPort(ObjectNode objectNode, String field, FieldPresence presence) { |
| 554 | - JsonNode node = objectNode.path(field); | 565 | + return isValid(objectNode, field, presence, n -> { |
| 555 | - return isValid(node, presence, node.isNumber() && | 566 | + TpPort.tpPort(n.asInt()); |
| 556 | - TpPort.tpPort(node.asInt()) != null); | 567 | + return true; |
| 568 | + }); | ||
| 557 | } | 569 | } |
| 558 | 570 | ||
| 559 | /** | 571 | /** |
| ... | @@ -562,8 +574,7 @@ public abstract class Config<S> { | ... | @@ -562,8 +574,7 @@ public abstract class Config<S> { |
| 562 | * @param field JSON field name | 574 | * @param field JSON field name |
| 563 | * @param presence specifies if field is optional or mandatory | 575 | * @param presence specifies if field is optional or mandatory |
| 564 | * @return true if valid; false otherwise | 576 | * @return true if valid; false otherwise |
| 565 | - * @throws IllegalArgumentException if field is present, but not valid | 577 | + * @throws InvalidFieldException if the field is present but not valid |
| 566 | - * connect point string representation | ||
| 567 | */ | 578 | */ |
| 568 | protected boolean isConnectPoint(String field, FieldPresence presence) { | 579 | protected boolean isConnectPoint(String field, FieldPresence presence) { |
| 569 | return isConnectPoint(object, field, presence); | 580 | return isConnectPoint(object, field, presence); |
| ... | @@ -577,13 +588,13 @@ public abstract class Config<S> { | ... | @@ -577,13 +588,13 @@ public abstract class Config<S> { |
| 577 | * @param field JSON field name | 588 | * @param field JSON field name |
| 578 | * @param presence specifies if field is optional or mandatory | 589 | * @param presence specifies if field is optional or mandatory |
| 579 | * @return true if valid; false otherwise | 590 | * @return true if valid; false otherwise |
| 580 | - * @throws IllegalArgumentException if field is present, but not valid | 591 | + * @throws InvalidFieldException if the field is present but not valid |
| 581 | - * connect point string representation | ||
| 582 | */ | 592 | */ |
| 583 | protected boolean isConnectPoint(ObjectNode objectNode, String field, FieldPresence presence) { | 593 | protected boolean isConnectPoint(ObjectNode objectNode, String field, FieldPresence presence) { |
| 584 | - JsonNode node = objectNode.path(field); | 594 | + return isValid(objectNode, field, presence, n -> { |
| 585 | - return isValid(node, presence, node.isTextual() && | 595 | + ConnectPoint.deviceConnectPoint(n.asText()); |
| 586 | - ConnectPoint.deviceConnectPoint(node.asText()) != null); | 596 | + return true; |
| 597 | + }); | ||
| 587 | } | 598 | } |
| 588 | 599 | ||
| 589 | /** | 600 | /** |
| ... | @@ -593,7 +604,7 @@ public abstract class Config<S> { | ... | @@ -593,7 +604,7 @@ public abstract class Config<S> { |
| 593 | * @param presence specifies if field is optional or mandatory | 604 | * @param presence specifies if field is optional or mandatory |
| 594 | * @param pattern optional regex pattern | 605 | * @param pattern optional regex pattern |
| 595 | * @return true if valid; false otherwise | 606 | * @return true if valid; false otherwise |
| 596 | - * @throws IllegalArgumentException if field is present, but not valid string | 607 | + * @throws InvalidFieldException if the field is present but not valid |
| 597 | */ | 608 | */ |
| 598 | protected boolean isString(String field, FieldPresence presence, String... pattern) { | 609 | protected boolean isString(String field, FieldPresence presence, String... pattern) { |
| 599 | return isString(object, field, presence, pattern); | 610 | return isString(object, field, presence, pattern); |
| ... | @@ -608,13 +619,17 @@ public abstract class Config<S> { | ... | @@ -608,13 +619,17 @@ public abstract class Config<S> { |
| 608 | * @param presence specifies if field is optional or mandatory | 619 | * @param presence specifies if field is optional or mandatory |
| 609 | * @param pattern optional regex pattern | 620 | * @param pattern optional regex pattern |
| 610 | * @return true if valid; false otherwise | 621 | * @return true if valid; false otherwise |
| 611 | - * @throws IllegalArgumentException if field is present, but not valid string | 622 | + * @throws InvalidFieldException if the field is present but not valid |
| 612 | */ | 623 | */ |
| 613 | protected boolean isString(ObjectNode objectNode, String field, | 624 | protected boolean isString(ObjectNode objectNode, String field, |
| 614 | FieldPresence presence, String... pattern) { | 625 | FieldPresence presence, String... pattern) { |
| 615 | - JsonNode node = objectNode.path(field); | 626 | + return isValid(objectNode, field, presence, (node) -> { |
| 616 | - return isValid(node, presence, node.isTextual() && | 627 | + if (!(node.isTextual() && |
| 617 | - (pattern.length > 0 && node.asText().matches(pattern[0]) || pattern.length < 1)); | 628 | + (pattern.length > 0 && node.asText().matches(pattern[0]) || pattern.length < 1))) { |
| 629 | + fail("Invalid string value"); | ||
| 630 | + } | ||
| 631 | + return true; | ||
| 632 | + }); | ||
| 618 | } | 633 | } |
| 619 | 634 | ||
| 620 | /** | 635 | /** |
| ... | @@ -624,28 +639,34 @@ public abstract class Config<S> { | ... | @@ -624,28 +639,34 @@ public abstract class Config<S> { |
| 624 | * @param presence specifies if field is optional or mandatory | 639 | * @param presence specifies if field is optional or mandatory |
| 625 | * @param minMax optional min/max values | 640 | * @param minMax optional min/max values |
| 626 | * @return true if valid; false otherwise | 641 | * @return true if valid; false otherwise |
| 627 | - * @throws IllegalArgumentException if field is present, but not valid | 642 | + * @throws InvalidFieldException if the field is present but not valid |
| 628 | */ | 643 | */ |
| 629 | protected boolean isNumber(String field, FieldPresence presence, long... minMax) { | 644 | protected boolean isNumber(String field, FieldPresence presence, long... minMax) { |
| 630 | - JsonNode node = object.path(field); | 645 | + return isNumber(object, field, presence, minMax); |
| 631 | - return isValid(node, presence, node.isNumber() && | ||
| 632 | - (minMax.length > 0 && minMax[0] <= node.asLong() || minMax.length < 1) && | ||
| 633 | - (minMax.length > 1 && minMax[1] > node.asLong() || minMax.length < 2)); | ||
| 634 | } | 646 | } |
| 647 | + | ||
| 635 | /** | 648 | /** |
| 636 | - * Indicates whether the specified field holds a valid number. | 649 | + * Indicates whether the specified field of a particular node holds a |
| 650 | + * valid number. | ||
| 637 | * | 651 | * |
| 652 | + * @param objectNode JSON object | ||
| 638 | * @param field JSON field name | 653 | * @param field JSON field name |
| 639 | * @param presence specifies if field is optional or mandatory | 654 | * @param presence specifies if field is optional or mandatory |
| 640 | * @param minMax optional min/max values | 655 | * @param minMax optional min/max values |
| 641 | * @return true if valid; false otherwise | 656 | * @return true if valid; false otherwise |
| 642 | - * @throws IllegalArgumentException if field is present, but not valid | 657 | + * @throws InvalidFieldException if the field is present but not valid |
| 643 | - */ | 658 | + */ |
| 644 | - protected boolean isNumber(String field, FieldPresence presence, double... minMax) { | 659 | + protected boolean isNumber(ObjectNode objectNode, String field, |
| 645 | - JsonNode node = object.path(field); | 660 | + FieldPresence presence, long... minMax) { |
| 646 | - return isValid(node, presence, node.isNumber() && | 661 | + return isValid(objectNode, field, presence, n -> { |
| 647 | - (minMax.length > 0 && minMax[0] <= node.asDouble() || minMax.length < 1) && | 662 | + long number = (n.isNumber()) ? n.asLong() : Long.parseLong(n.asText()); |
| 648 | - (minMax.length > 1 && minMax[1] > node.asDouble() || minMax.length < 2)); | 663 | + if (minMax.length > 1) { |
| 664 | + verifyRange(number, minMax[0], minMax[1]); | ||
| 665 | + } else if (minMax.length > 0) { | ||
| 666 | + verifyRange(number, minMax[0]); | ||
| 667 | + } | ||
| 668 | + return true; | ||
| 669 | + }); | ||
| 649 | } | 670 | } |
| 650 | 671 | ||
| 651 | /** | 672 | /** |
| ... | @@ -655,7 +676,7 @@ public abstract class Config<S> { | ... | @@ -655,7 +676,7 @@ public abstract class Config<S> { |
| 655 | * @param presence specifies if field is optional or mandatory | 676 | * @param presence specifies if field is optional or mandatory |
| 656 | * @param minMax optional min/max values | 677 | * @param minMax optional min/max values |
| 657 | * @return true if valid; false otherwise | 678 | * @return true if valid; false otherwise |
| 658 | - * @throws IllegalArgumentException if field is present, but not valid | 679 | + * @throws InvalidFieldException if the field is present but not valid |
| 659 | */ | 680 | */ |
| 660 | protected boolean isIntegralNumber(String field, FieldPresence presence, long... minMax) { | 681 | protected boolean isIntegralNumber(String field, FieldPresence presence, long... minMax) { |
| 661 | return isIntegralNumber(object, field, presence, minMax); | 682 | return isIntegralNumber(object, field, presence, minMax); |
| ... | @@ -670,16 +691,18 @@ public abstract class Config<S> { | ... | @@ -670,16 +691,18 @@ public abstract class Config<S> { |
| 670 | * @param presence specifies if field is optional or mandatory | 691 | * @param presence specifies if field is optional or mandatory |
| 671 | * @param minMax optional min/max values | 692 | * @param minMax optional min/max values |
| 672 | * @return true if valid; false otherwise | 693 | * @return true if valid; false otherwise |
| 673 | - * @throws IllegalArgumentException if field is present, but not valid | 694 | + * @throws InvalidFieldException if the field is present but not valid |
| 674 | */ | 695 | */ |
| 675 | protected boolean isIntegralNumber(ObjectNode objectNode, String field, | 696 | protected boolean isIntegralNumber(ObjectNode objectNode, String field, |
| 676 | FieldPresence presence, long... minMax) { | 697 | FieldPresence presence, long... minMax) { |
| 677 | - JsonNode node = objectNode.path(field); | 698 | + return isValid(objectNode, field, presence, n -> { |
| 678 | - | 699 | + long number = (n.isIntegralNumber()) ? n.asLong() : Long.parseLong(n.asText()); |
| 679 | - return isValid(node, presence, n -> { | 700 | + if (minMax.length > 1) { |
| 680 | - long number = (node.isIntegralNumber()) ? n.asLong() : Long.parseLong(n.asText()); | 701 | + verifyRange(number, minMax[0], minMax[1]); |
| 681 | - return (minMax.length > 0 && minMax[0] <= number || minMax.length < 1) && | 702 | + } else if (minMax.length > 0) { |
| 682 | - (minMax.length > 1 && minMax[1] > number || minMax.length < 2); | 703 | + verifyRange(number, minMax[0]); |
| 704 | + } | ||
| 705 | + return true; | ||
| 683 | }); | 706 | }); |
| 684 | } | 707 | } |
| 685 | 708 | ||
| ... | @@ -690,13 +713,34 @@ public abstract class Config<S> { | ... | @@ -690,13 +713,34 @@ public abstract class Config<S> { |
| 690 | * @param presence specifies if field is optional or mandatory | 713 | * @param presence specifies if field is optional or mandatory |
| 691 | * @param minMax optional min/max values | 714 | * @param minMax optional min/max values |
| 692 | * @return true if valid; false otherwise | 715 | * @return true if valid; false otherwise |
| 693 | - * @throws IllegalArgumentException if field is present, but not valid | 716 | + * @throws InvalidFieldException if the field is present but not valid |
| 694 | */ | 717 | */ |
| 695 | protected boolean isDecimal(String field, FieldPresence presence, double... minMax) { | 718 | protected boolean isDecimal(String field, FieldPresence presence, double... minMax) { |
| 696 | - JsonNode node = object.path(field); | 719 | + return isDecimal(object, field, presence, minMax); |
| 697 | - return isValid(node, presence, (node.isDouble() || node.isFloat()) && | 720 | + } |
| 698 | - (minMax.length > 0 && minMax[0] <= node.asDouble() || minMax.length < 1) && | 721 | + |
| 699 | - (minMax.length > 1 && minMax[1] > node.asDouble() || minMax.length < 2)); | 722 | + /** |
| 723 | + * Indicates whether the specified field of a particular node holds a valid | ||
| 724 | + * decimal number. | ||
| 725 | + * | ||
| 726 | + * @param objectNode JSON node | ||
| 727 | + * @param field JSON field name | ||
| 728 | + * @param presence specifies if field is optional or mandatory | ||
| 729 | + * @param minMax optional min/max values | ||
| 730 | + * @return true if valid; false otherwise | ||
| 731 | + * @throws InvalidFieldException if the field is present but not valid | ||
| 732 | + */ | ||
| 733 | + protected boolean isDecimal(ObjectNode objectNode, String field, | ||
| 734 | + FieldPresence presence, double... minMax) { | ||
| 735 | + return isValid(objectNode, field, presence, n -> { | ||
| 736 | + double number = (n.isDouble()) ? n.asDouble() : Double.parseDouble(n.asText()); | ||
| 737 | + if (minMax.length > 1) { | ||
| 738 | + verifyRange(number, minMax[0], minMax[1]); | ||
| 739 | + } else if (minMax.length > 0) { | ||
| 740 | + verifyRange(number, minMax[0]); | ||
| 741 | + } | ||
| 742 | + return true; | ||
| 743 | + }); | ||
| 700 | } | 744 | } |
| 701 | 745 | ||
| 702 | /** | 746 | /** |
| ... | @@ -705,6 +749,7 @@ public abstract class Config<S> { | ... | @@ -705,6 +749,7 @@ public abstract class Config<S> { |
| 705 | * @param field JSON field name | 749 | * @param field JSON field name |
| 706 | * @param presence specifies if field is optional or mandatory | 750 | * @param presence specifies if field is optional or mandatory |
| 707 | * @return true if valid; false otherwise | 751 | * @return true if valid; false otherwise |
| 752 | + * @throws InvalidFieldException if the field is present but not valid | ||
| 708 | */ | 753 | */ |
| 709 | protected boolean isBoolean(String field, FieldPresence presence) { | 754 | protected boolean isBoolean(String field, FieldPresence presence) { |
| 710 | return isBoolean(object, field, presence); | 755 | return isBoolean(object, field, presence); |
| ... | @@ -718,11 +763,15 @@ public abstract class Config<S> { | ... | @@ -718,11 +763,15 @@ public abstract class Config<S> { |
| 718 | * @param field JSON field name | 763 | * @param field JSON field name |
| 719 | * @param presence specifies if field is optional or mandatory | 764 | * @param presence specifies if field is optional or mandatory |
| 720 | * @return true if valid; false otherwise | 765 | * @return true if valid; false otherwise |
| 766 | + * @throws InvalidFieldException if the field is present but not valid | ||
| 721 | */ | 767 | */ |
| 722 | protected boolean isBoolean(ObjectNode objectNode, String field, FieldPresence presence) { | 768 | protected boolean isBoolean(ObjectNode objectNode, String field, FieldPresence presence) { |
| 723 | - JsonNode node = objectNode.path(field); | 769 | + return isValid(objectNode, field, presence, n -> { |
| 724 | - return isValid(node, presence, node.isBoolean() || | 770 | + if (!(n.isBoolean() || (n.isTextual() && isBooleanString(n.asText())))) { |
| 725 | - (node.isTextual() && isBooleanString(node.asText()))); | 771 | + fail("Field is not a boolean value"); |
| 772 | + } | ||
| 773 | + return true; | ||
| 774 | + }); | ||
| 726 | } | 775 | } |
| 727 | 776 | ||
| 728 | /** | 777 | /** |
| ... | @@ -737,39 +786,56 @@ public abstract class Config<S> { | ... | @@ -737,39 +786,56 @@ public abstract class Config<S> { |
| 737 | } | 786 | } |
| 738 | 787 | ||
| 739 | /** | 788 | /** |
| 740 | - * Indicates whether the node is present and of correct value or not | 789 | + * Indicates whether a field in the node is present and of correct value or |
| 741 | - * mandatory and absent. | 790 | + * not mandatory and absent. |
| 742 | * | 791 | * |
| 743 | - * @param node JSON node | 792 | + * @param objectNode JSON object node containing field to validate |
| 744 | - * @param presence specifies if field is optional or mandatory | 793 | + * @param field name of field to validate |
| 745 | - * @param correctValue true if the value is correct | ||
| 746 | - * @return true if the field is as expected | ||
| 747 | - */ | ||
| 748 | - private boolean isValid(JsonNode node, FieldPresence presence, boolean correctValue) { | ||
| 749 | - return isValid(node, presence, n -> correctValue); | ||
| 750 | - } | ||
| 751 | - | ||
| 752 | - /** | ||
| 753 | - * Indicates whether the node is present and of correct value or not | ||
| 754 | - * mandatory and absent. | ||
| 755 | - * | ||
| 756 | - * @param node JSON node | ||
| 757 | * @param presence specified if field is optional or mandatory | 794 | * @param presence specified if field is optional or mandatory |
| 758 | * @param validationFunction function which can be used to verify if the | 795 | * @param validationFunction function which can be used to verify if the |
| 759 | * node has the correct value | 796 | * node has the correct value |
| 760 | * @return true if the field is as expected | 797 | * @return true if the field is as expected |
| 798 | + * @throws InvalidFieldException if the field is present but not valid | ||
| 761 | */ | 799 | */ |
| 762 | - private boolean isValid(JsonNode node, FieldPresence presence, | 800 | + private boolean isValid(ObjectNode objectNode, String field, FieldPresence presence, |
| 763 | Function<JsonNode, Boolean> validationFunction) { | 801 | Function<JsonNode, Boolean> validationFunction) { |
| 802 | + JsonNode node = objectNode.path(field); | ||
| 764 | boolean isMandatory = presence == FieldPresence.MANDATORY; | 803 | boolean isMandatory = presence == FieldPresence.MANDATORY; |
| 765 | - if (isMandatory && validationFunction.apply(node)) { | 804 | + if (isMandatory && node.isMissingNode()) { |
| 766 | - return true; | 805 | + throw new InvalidFieldException(field, "Mandatory field not present"); |
| 767 | } | 806 | } |
| 768 | 807 | ||
| 769 | if (!isMandatory && (node.isNull() || node.isMissingNode())) { | 808 | if (!isMandatory && (node.isNull() || node.isMissingNode())) { |
| 770 | return true; | 809 | return true; |
| 771 | } | 810 | } |
| 772 | 811 | ||
| 773 | - return validationFunction.apply(node); | 812 | + try { |
| 813 | + if (validationFunction.apply(node)) { | ||
| 814 | + return true; | ||
| 815 | + } else { | ||
| 816 | + throw new InvalidFieldException(field, "Validation error"); | ||
| 817 | + } | ||
| 818 | + } catch (IllegalArgumentException e) { | ||
| 819 | + throw new InvalidFieldException(field, e); | ||
| 820 | + } | ||
| 821 | + } | ||
| 822 | + | ||
| 823 | + private static void fail(String message) { | ||
| 824 | + throw new IllegalArgumentException(message); | ||
| 825 | + } | ||
| 826 | + | ||
| 827 | + private static <N extends Comparable> void verifyRange(N num, N min) { | ||
| 828 | + if (num.compareTo(min) < 0) { | ||
| 829 | + fail("Field must be greater than " + min); | ||
| 830 | + } | ||
| 831 | + } | ||
| 832 | + | ||
| 833 | + private static <N extends Comparable> void verifyRange(N num, N min, N max) { | ||
| 834 | + verifyRange(num, min); | ||
| 835 | + | ||
| 836 | + if (num.compareTo(max) > 0) { | ||
| 837 | + fail("Field must be less than " + max); | ||
| 838 | + } | ||
| 774 | } | 839 | } |
| 840 | + | ||
| 775 | } | 841 | } | ... | ... |
| 1 | +/* | ||
| 2 | + * Copyright 2016 Open Networking Laboratory | ||
| 3 | + * | ||
| 4 | + * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| 5 | + * you may not use this file except in compliance with the License. | ||
| 6 | + * You may obtain a copy of the License at | ||
| 7 | + * | ||
| 8 | + * http://www.apache.org/licenses/LICENSE-2.0 | ||
| 9 | + * | ||
| 10 | + * Unless required by applicable law or agreed to in writing, software | ||
| 11 | + * distributed under the License is distributed on an "AS IS" BASIS, | ||
| 12 | + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| 13 | + * See the License for the specific language governing permissions and | ||
| 14 | + * limitations under the License. | ||
| 15 | + */ | ||
| 16 | + | ||
| 17 | +package org.onosproject.net.config; | ||
| 18 | + | ||
| 19 | +/** | ||
| 20 | + * Indicates an invalid configuration was supplied by the user. | ||
| 21 | + */ | ||
| 22 | +public class InvalidConfigException extends RuntimeException { | ||
| 23 | + | ||
| 24 | + private final String subjectKey; | ||
| 25 | + private final String subject; | ||
| 26 | + private final String configKey; | ||
| 27 | + | ||
| 28 | + /** | ||
| 29 | + * Creates a new invalid config exception about a specific config. | ||
| 30 | + * | ||
| 31 | + * @param subjectKey config's subject key | ||
| 32 | + * @param subject config's subject | ||
| 33 | + * @param configKey config's config key | ||
| 34 | + */ | ||
| 35 | + public InvalidConfigException(String subjectKey, String subject, String configKey) { | ||
| 36 | + this(subjectKey, subject, configKey, null); | ||
| 37 | + } | ||
| 38 | + | ||
| 39 | + /** | ||
| 40 | + * Creates a new invalid config exception about a specific config with an | ||
| 41 | + * exception regarding the cause of the invalidity. | ||
| 42 | + * | ||
| 43 | + * @param subjectKey config's subject key | ||
| 44 | + * @param subject config's subject | ||
| 45 | + * @param configKey config's config key | ||
| 46 | + * @param cause cause of the invalidity | ||
| 47 | + */ | ||
| 48 | + public InvalidConfigException(String subjectKey, String subject, String configKey, Throwable cause) { | ||
| 49 | + super(message(subjectKey, subject, configKey, cause), cause); | ||
| 50 | + this.subjectKey = subjectKey; | ||
| 51 | + this.subject = subject; | ||
| 52 | + this.configKey = configKey; | ||
| 53 | + } | ||
| 54 | + | ||
| 55 | + /** | ||
| 56 | + * Returns the subject key of the config. | ||
| 57 | + * | ||
| 58 | + * @return subject key | ||
| 59 | + */ | ||
| 60 | + public String subjectKey() { | ||
| 61 | + return subjectKey; | ||
| 62 | + } | ||
| 63 | + | ||
| 64 | + /** | ||
| 65 | + * Returns the string representation of the subject of the config. | ||
| 66 | + * | ||
| 67 | + * @return subject | ||
| 68 | + */ | ||
| 69 | + public String subject() { | ||
| 70 | + return subject; | ||
| 71 | + } | ||
| 72 | + | ||
| 73 | + /** | ||
| 74 | + * Returns the config key of the config. | ||
| 75 | + * | ||
| 76 | + * @return config key | ||
| 77 | + */ | ||
| 78 | + public String configKey() { | ||
| 79 | + return configKey; | ||
| 80 | + } | ||
| 81 | + | ||
| 82 | + private static String message(String subjectKey, String subject, String configKey, Throwable cause) { | ||
| 83 | + String error = "Error parsing config " + subjectKey + "/" + subject + "/" + configKey; | ||
| 84 | + if (cause != null) { | ||
| 85 | + error = error + ": " + cause.getMessage(); | ||
| 86 | + } | ||
| 87 | + return error; | ||
| 88 | + } | ||
| 89 | +} |
| 1 | +/* | ||
| 2 | + * Copyright 2016-present Open Networking Laboratory | ||
| 3 | + * | ||
| 4 | + * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| 5 | + * you may not use this file except in compliance with the License. | ||
| 6 | + * You may obtain a copy of the License at | ||
| 7 | + * | ||
| 8 | + * http://www.apache.org/licenses/LICENSE-2.0 | ||
| 9 | + * | ||
| 10 | + * Unless required by applicable law or agreed to in writing, software | ||
| 11 | + * distributed under the License is distributed on an "AS IS" BASIS, | ||
| 12 | + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| 13 | + * See the License for the specific language governing permissions and | ||
| 14 | + * limitations under the License. | ||
| 15 | + */ | ||
| 16 | + | ||
| 17 | +package org.onosproject.net.config; | ||
| 18 | + | ||
| 19 | +/** | ||
| 20 | + * Indicates a field of a configuration was invalid. | ||
| 21 | + */ | ||
| 22 | +public class InvalidFieldException extends RuntimeException { | ||
| 23 | + | ||
| 24 | + private final String field; | ||
| 25 | + private final String reason; | ||
| 26 | + | ||
| 27 | + /** | ||
| 28 | + * Creates a new invalid field exception about a given field. | ||
| 29 | + * | ||
| 30 | + * @param field field name | ||
| 31 | + * @param reason reason the field is invalid | ||
| 32 | + */ | ||
| 33 | + public InvalidFieldException(String field, String reason) { | ||
| 34 | + super(message(field, reason)); | ||
| 35 | + this.field = field; | ||
| 36 | + this.reason = reason; | ||
| 37 | + } | ||
| 38 | + | ||
| 39 | + /** | ||
| 40 | + * Creates a new invalid field exception about a given field. | ||
| 41 | + * | ||
| 42 | + * @param field field name | ||
| 43 | + * @param cause throwable that occurred while trying to validate field | ||
| 44 | + */ | ||
| 45 | + public InvalidFieldException(String field, Throwable cause) { | ||
| 46 | + super(message(field, cause.getMessage())); | ||
| 47 | + this.field = field; | ||
| 48 | + this.reason = cause.getMessage(); | ||
| 49 | + } | ||
| 50 | + | ||
| 51 | + /** | ||
| 52 | + * Returns the field name. | ||
| 53 | + * | ||
| 54 | + * @return field name | ||
| 55 | + */ | ||
| 56 | + public String field() { | ||
| 57 | + return field; | ||
| 58 | + } | ||
| 59 | + | ||
| 60 | + /** | ||
| 61 | + * Returns the reason the field failed to validate. | ||
| 62 | + * | ||
| 63 | + * @return reason | ||
| 64 | + */ | ||
| 65 | + public String reason() { | ||
| 66 | + return reason; | ||
| 67 | + } | ||
| 68 | + | ||
| 69 | + private static String message(String field, String reason) { | ||
| 70 | + return "Field \"" + field + "\" is invalid: " + reason; | ||
| 71 | + } | ||
| 72 | + | ||
| 73 | +} |
| ... | @@ -21,7 +21,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; | ... | @@ -21,7 +21,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; |
| 21 | import org.junit.Before; | 21 | import org.junit.Before; |
| 22 | import org.junit.Test; | 22 | import org.junit.Test; |
| 23 | 23 | ||
| 24 | -import static org.junit.Assert.assertFalse; | ||
| 25 | import static org.junit.Assert.assertTrue; | 24 | import static org.junit.Assert.assertTrue; |
| 26 | import static org.onosproject.net.config.Config.FieldPresence.MANDATORY; | 25 | import static org.onosproject.net.config.Config.FieldPresence.MANDATORY; |
| 27 | import static org.onosproject.net.config.Config.FieldPresence.OPTIONAL; | 26 | import static org.onosproject.net.config.Config.FieldPresence.OPTIONAL; |
| ... | @@ -37,8 +36,15 @@ public class ConfigTest { | ... | @@ -37,8 +36,15 @@ public class ConfigTest { |
| 37 | private static final String TEXT = "text"; | 36 | private static final String TEXT = "text"; |
| 38 | private static final String LONG = "long"; | 37 | private static final String LONG = "long"; |
| 39 | private static final String DOUBLE = "double"; | 38 | private static final String DOUBLE = "double"; |
| 39 | + private static final String BOOLEAN = "boolean"; | ||
| 40 | private static final String MAC = "mac"; | 40 | private static final String MAC = "mac"; |
| 41 | + private static final String BAD_MAC = "badMac"; | ||
| 41 | private static final String IP = "ip"; | 42 | private static final String IP = "ip"; |
| 43 | + private static final String BAD_IP = "badIp"; | ||
| 44 | + private static final String PREFIX = "prefix"; | ||
| 45 | + private static final String BAD_PREFIX = "badPrefix"; | ||
| 46 | + private static final String CONNECT_POINT = "connectPoint"; | ||
| 47 | + private static final String BAD_CONNECT_POINT = "badConnectPoint"; | ||
| 42 | private static final String TP_PORT = "tpPort"; | 48 | private static final String TP_PORT = "tpPort"; |
| 43 | private static final String BAD_TP_PORT = "badTpPort"; | 49 | private static final String BAD_TP_PORT = "badTpPort"; |
| 44 | 50 | ||
| ... | @@ -52,7 +58,12 @@ public class ConfigTest { | ... | @@ -52,7 +58,12 @@ public class ConfigTest { |
| 52 | public void setUp() { | 58 | public void setUp() { |
| 53 | json = new ObjectMapper().createObjectNode() | 59 | json = new ObjectMapper().createObjectNode() |
| 54 | .put(TEXT, "foo").put(LONG, 5).put(DOUBLE, 0.5) | 60 | .put(TEXT, "foo").put(LONG, 5).put(DOUBLE, 0.5) |
| 55 | - .put(MAC, "ab:cd:ef:ca:fe:ed").put(IP, "12.34.56.78") | 61 | + .put(BOOLEAN, "true") |
| 62 | + .put(MAC, "ab:cd:ef:ca:fe:ed").put(BAD_MAC, "ab:cd:ef:ca:fe.ed") | ||
| 63 | + .put(IP, "12.34.56.78").put(BAD_IP, "12.34-56.78") | ||
| 64 | + .put(PREFIX, "12.34.56.78/18").put(BAD_PREFIX, "12.34.56.78-18") | ||
| 65 | + .put(CONNECT_POINT, "of:0000000000000001/1") | ||
| 66 | + .put(BAD_CONNECT_POINT, "of:0000000000000001-1") | ||
| 56 | .put(TP_PORT, 65535).put(BAD_TP_PORT, 65536); | 67 | .put(TP_PORT, 65535).put(BAD_TP_PORT, 65536); |
| 57 | cfg = new TestConfig(); | 68 | cfg = new TestConfig(); |
| 58 | cfg.init(SUBJECT, KEY, json, mapper, delegate); | 69 | cfg.init(SUBJECT, KEY, json, mapper, delegate); |
| ... | @@ -60,16 +71,20 @@ public class ConfigTest { | ... | @@ -60,16 +71,20 @@ public class ConfigTest { |
| 60 | 71 | ||
| 61 | @Test | 72 | @Test |
| 62 | public void hasOnlyFields() { | 73 | public void hasOnlyFields() { |
| 63 | - assertTrue("has unexpected fields", cfg.hasOnlyFields(TEXT, LONG, DOUBLE, MAC, IP, | 74 | + assertTrue("has unexpected fields", |
| 64 | - TP_PORT, BAD_TP_PORT)); | 75 | + cfg.hasOnlyFields(TEXT, LONG, DOUBLE, BOOLEAN, MAC, BAD_MAC, |
| 65 | - assertFalse("did not detect unexpected fields", cfg.hasOnlyFields(TEXT, LONG, DOUBLE, MAC)); | 76 | + IP, BAD_IP, PREFIX, BAD_PREFIX, |
| 66 | - assertTrue("is not proper text", cfg.isString(TEXT, MANDATORY)); | 77 | + CONNECT_POINT, BAD_CONNECT_POINT, TP_PORT, BAD_TP_PORT)); |
| 78 | + assertTrue("did not detect unexpected fields", | ||
| 79 | + expectInvalidField(() -> cfg.hasOnlyFields(TEXT, LONG, DOUBLE, MAC))); | ||
| 67 | } | 80 | } |
| 68 | 81 | ||
| 69 | @Test | 82 | @Test |
| 70 | public void hasFields() { | 83 | public void hasFields() { |
| 71 | - assertTrue("does not have mandatory field", cfg.hasFields(TEXT, LONG, DOUBLE, MAC)); | 84 | + assertTrue("does not have mandatory field", |
| 72 | - assertFalse("did not detect missing field", cfg.hasFields("none")); | 85 | + cfg.hasFields(TEXT, LONG, DOUBLE, MAC)); |
| 86 | + assertTrue("did not detect missing field", | ||
| 87 | + expectInvalidField(() -> cfg.hasFields("none"))); | ||
| 73 | } | 88 | } |
| 74 | 89 | ||
| 75 | @Test | 90 | @Test |
| ... | @@ -79,7 +94,10 @@ public class ConfigTest { | ... | @@ -79,7 +94,10 @@ public class ConfigTest { |
| 79 | assertTrue("is not proper text", cfg.isString(TEXT, OPTIONAL, "^f.*")); | 94 | assertTrue("is not proper text", cfg.isString(TEXT, OPTIONAL, "^f.*")); |
| 80 | assertTrue("is not proper text", cfg.isString(TEXT, OPTIONAL)); | 95 | assertTrue("is not proper text", cfg.isString(TEXT, OPTIONAL)); |
| 81 | assertTrue("is not proper text", cfg.isString("none", OPTIONAL)); | 96 | assertTrue("is not proper text", cfg.isString("none", OPTIONAL)); |
| 82 | - assertFalse("did not detect missing field", cfg.isString("none", MANDATORY)); | 97 | + assertTrue("did not detect missing field", |
| 98 | + expectInvalidField(() -> cfg.isString("none", MANDATORY))); | ||
| 99 | + assertTrue("did not detect bad text", | ||
| 100 | + expectInvalidField(() -> cfg.isString(TEXT, OPTIONAL, "^b.*"))); | ||
| 83 | } | 101 | } |
| 84 | 102 | ||
| 85 | @Test | 103 | @Test |
| ... | @@ -88,12 +106,41 @@ public class ConfigTest { | ... | @@ -88,12 +106,41 @@ public class ConfigTest { |
| 88 | assertTrue("is not proper number", cfg.isNumber(LONG, MANDATORY, 0)); | 106 | assertTrue("is not proper number", cfg.isNumber(LONG, MANDATORY, 0)); |
| 89 | assertTrue("is not proper number", cfg.isNumber(LONG, MANDATORY, 0, 10)); | 107 | assertTrue("is not proper number", cfg.isNumber(LONG, MANDATORY, 0, 10)); |
| 90 | assertTrue("is not proper number", cfg.isNumber(LONG, MANDATORY, 5, 6)); | 108 | assertTrue("is not proper number", cfg.isNumber(LONG, MANDATORY, 5, 6)); |
| 91 | - assertFalse("is not in range", cfg.isNumber(LONG, MANDATORY, 6, 10)); | 109 | + assertTrue("is not in range", |
| 92 | - assertFalse("is not in range", cfg.isNumber(LONG, MANDATORY, 4, 5)); | 110 | + expectInvalidField(() -> cfg.isNumber(LONG, MANDATORY, 6, 10))); |
| 111 | + assertTrue("is not in range", cfg.isNumber(LONG, MANDATORY, 4, 5)); | ||
| 93 | assertTrue("is not proper number", cfg.isNumber(LONG, OPTIONAL, 0, 10)); | 112 | assertTrue("is not proper number", cfg.isNumber(LONG, OPTIONAL, 0, 10)); |
| 94 | assertTrue("is not proper number", cfg.isNumber(LONG, OPTIONAL)); | 113 | assertTrue("is not proper number", cfg.isNumber(LONG, OPTIONAL)); |
| 95 | assertTrue("is not proper number", cfg.isNumber("none", OPTIONAL)); | 114 | assertTrue("is not proper number", cfg.isNumber("none", OPTIONAL)); |
| 96 | - assertFalse("did not detect missing field", cfg.isNumber("none", MANDATORY)); | 115 | + assertTrue("did not detect missing field", |
| 116 | + expectInvalidField(() -> cfg.isNumber("none", MANDATORY))); | ||
| 117 | + assertTrue("is not proper number", | ||
| 118 | + expectInvalidField(() -> cfg.isNumber(TEXT, MANDATORY))); | ||
| 119 | + | ||
| 120 | + assertTrue("is not proper number", cfg.isNumber(DOUBLE, MANDATORY, 0, 1)); | ||
| 121 | + assertTrue("is not in range", | ||
| 122 | + expectInvalidField(() -> cfg.isNumber(DOUBLE, MANDATORY, 1, 2))); | ||
| 123 | + } | ||
| 124 | + | ||
| 125 | + @Test | ||
| 126 | + public void isIntegralNumber() { | ||
| 127 | + assertTrue("is not proper number", cfg.isIntegralNumber(LONG, MANDATORY)); | ||
| 128 | + assertTrue("is not proper number", cfg.isIntegralNumber(LONG, MANDATORY, 0)); | ||
| 129 | + assertTrue("is not proper number", cfg.isIntegralNumber(LONG, MANDATORY, 0, 10)); | ||
| 130 | + assertTrue("is not proper number", cfg.isIntegralNumber(LONG, MANDATORY, 5, 6)); | ||
| 131 | + assertTrue("is not in range", | ||
| 132 | + expectInvalidField(() -> cfg.isIntegralNumber(LONG, MANDATORY, 6, 10))); | ||
| 133 | + assertTrue("is not in range", cfg.isIntegralNumber(LONG, MANDATORY, 4, 5)); | ||
| 134 | + assertTrue("is not proper number", cfg.isIntegralNumber(LONG, OPTIONAL, 0, 10)); | ||
| 135 | + assertTrue("is not proper number", cfg.isIntegralNumber(LONG, OPTIONAL)); | ||
| 136 | + assertTrue("is not proper number", cfg.isIntegralNumber("none", OPTIONAL)); | ||
| 137 | + assertTrue("did not detect missing field", | ||
| 138 | + expectInvalidField(() -> cfg.isIntegralNumber("none", MANDATORY))); | ||
| 139 | + assertTrue("is not proper number", | ||
| 140 | + expectInvalidField(() -> cfg.isIntegralNumber(TEXT, MANDATORY))); | ||
| 141 | + | ||
| 142 | + assertTrue("is not in range", | ||
| 143 | + expectInvalidField(() -> cfg.isIntegralNumber(DOUBLE, MANDATORY, 0, 10))); | ||
| 97 | } | 144 | } |
| 98 | 145 | ||
| 99 | @Test | 146 | @Test |
| ... | @@ -102,12 +149,26 @@ public class ConfigTest { | ... | @@ -102,12 +149,26 @@ public class ConfigTest { |
| 102 | assertTrue("is not proper decimal", cfg.isDecimal(DOUBLE, MANDATORY, 0.0)); | 149 | assertTrue("is not proper decimal", cfg.isDecimal(DOUBLE, MANDATORY, 0.0)); |
| 103 | assertTrue("is not proper decimal", cfg.isDecimal(DOUBLE, MANDATORY, 0.0, 1.0)); | 150 | assertTrue("is not proper decimal", cfg.isDecimal(DOUBLE, MANDATORY, 0.0, 1.0)); |
| 104 | assertTrue("is not proper decimal", cfg.isDecimal(DOUBLE, MANDATORY, 0.5, 0.6)); | 151 | assertTrue("is not proper decimal", cfg.isDecimal(DOUBLE, MANDATORY, 0.5, 0.6)); |
| 105 | - assertFalse("is not in range", cfg.isDecimal(DOUBLE, MANDATORY, 0.6, 1.0)); | 152 | + assertTrue("is not proper decimal", cfg.isDecimal(DOUBLE, MANDATORY, 0.4, 0.5)); |
| 106 | - assertFalse("is not in range", cfg.isDecimal(DOUBLE, MANDATORY, 0.4, 0.5)); | 153 | + assertTrue("is not in range", |
| 154 | + expectInvalidField(() -> cfg.isDecimal(DOUBLE, MANDATORY, 0.6, 1.0))); | ||
| 155 | + assertTrue("is not in range", | ||
| 156 | + expectInvalidField(() -> cfg.isDecimal(DOUBLE, MANDATORY, 0.3, 0.4))); | ||
| 107 | assertTrue("is not proper decimal", cfg.isDecimal(DOUBLE, OPTIONAL, 0.0, 1.0)); | 157 | assertTrue("is not proper decimal", cfg.isDecimal(DOUBLE, OPTIONAL, 0.0, 1.0)); |
| 108 | assertTrue("is not proper decimal", cfg.isDecimal(DOUBLE, OPTIONAL)); | 158 | assertTrue("is not proper decimal", cfg.isDecimal(DOUBLE, OPTIONAL)); |
| 109 | assertTrue("is not proper decimal", cfg.isDecimal("none", OPTIONAL)); | 159 | assertTrue("is not proper decimal", cfg.isDecimal("none", OPTIONAL)); |
| 110 | - assertFalse("did not detect missing field", cfg.isDecimal("none", MANDATORY)); | 160 | + assertTrue("did not detect missing field", |
| 161 | + expectInvalidField(() -> cfg.isDecimal("none", MANDATORY))); | ||
| 162 | + } | ||
| 163 | + | ||
| 164 | + @Test | ||
| 165 | + public void isBoolean() { | ||
| 166 | + assertTrue("is not proper boolean", cfg.isBoolean(BOOLEAN, MANDATORY)); | ||
| 167 | + assertTrue("did not detect missing field", | ||
| 168 | + expectInvalidField(() -> cfg.isBoolean("none", MANDATORY))); | ||
| 169 | + assertTrue("is not proper boolean", cfg.isBoolean("none", OPTIONAL)); | ||
| 170 | + assertTrue("did not detect bad boolean", | ||
| 171 | + expectInvalidField(() -> cfg.isBoolean(TEXT, MANDATORY))); | ||
| 111 | } | 172 | } |
| 112 | 173 | ||
| 113 | @Test | 174 | @Test |
| ... | @@ -115,26 +176,43 @@ public class ConfigTest { | ... | @@ -115,26 +176,43 @@ public class ConfigTest { |
| 115 | assertTrue("is not proper mac", cfg.isMacAddress(MAC, MANDATORY)); | 176 | assertTrue("is not proper mac", cfg.isMacAddress(MAC, MANDATORY)); |
| 116 | assertTrue("is not proper mac", cfg.isMacAddress(MAC, OPTIONAL)); | 177 | assertTrue("is not proper mac", cfg.isMacAddress(MAC, OPTIONAL)); |
| 117 | assertTrue("is not proper mac", cfg.isMacAddress("none", OPTIONAL)); | 178 | assertTrue("is not proper mac", cfg.isMacAddress("none", OPTIONAL)); |
| 118 | - assertFalse("did not detect missing field", cfg.isMacAddress("none", MANDATORY)); | 179 | + assertTrue("did not detect missing field", |
| 119 | - } | 180 | + expectInvalidField(() -> cfg.isMacAddress("none", MANDATORY))); |
| 120 | - | 181 | + assertTrue("did not detect bad ip", |
| 121 | - @Test(expected = IllegalArgumentException.class) | 182 | + expectInvalidField(() -> cfg.isMacAddress(BAD_MAC, MANDATORY))); |
| 122 | - public void badMacAddress() { | ||
| 123 | - assertTrue("is not proper mac", cfg.isMacAddress(TEXT, MANDATORY)); | ||
| 124 | } | 183 | } |
| 125 | 184 | ||
| 126 | - | ||
| 127 | @Test | 185 | @Test |
| 128 | public void isIpAddress() { | 186 | public void isIpAddress() { |
| 129 | assertTrue("is not proper ip", cfg.isIpAddress(IP, MANDATORY)); | 187 | assertTrue("is not proper ip", cfg.isIpAddress(IP, MANDATORY)); |
| 130 | assertTrue("is not proper ip", cfg.isIpAddress(IP, OPTIONAL)); | 188 | assertTrue("is not proper ip", cfg.isIpAddress(IP, OPTIONAL)); |
| 131 | assertTrue("is not proper ip", cfg.isIpAddress("none", OPTIONAL)); | 189 | assertTrue("is not proper ip", cfg.isIpAddress("none", OPTIONAL)); |
| 132 | - assertFalse("did not detect missing field", cfg.isMacAddress("none", MANDATORY)); | 190 | + assertTrue("did not detect missing ip", |
| 191 | + expectInvalidField(() -> cfg.isIpAddress("none", MANDATORY))); | ||
| 192 | + assertTrue("did not detect bad ip", | ||
| 193 | + expectInvalidField(() -> cfg.isIpAddress(BAD_IP, MANDATORY))); | ||
| 194 | + } | ||
| 195 | + | ||
| 196 | + @Test | ||
| 197 | + public void isIpPrefix() { | ||
| 198 | + assertTrue("is not proper prefix", cfg.isIpPrefix(PREFIX, MANDATORY)); | ||
| 199 | + assertTrue("is not proper prefix", cfg.isIpPrefix(PREFIX, OPTIONAL)); | ||
| 200 | + assertTrue("is not proper prefix", cfg.isIpPrefix("none", OPTIONAL)); | ||
| 201 | + assertTrue("did not detect missing prefix", | ||
| 202 | + expectInvalidField(() -> cfg.isIpPrefix("none", MANDATORY))); | ||
| 203 | + assertTrue("did not detect bad prefix", | ||
| 204 | + expectInvalidField(() -> cfg.isIpPrefix(BAD_PREFIX, MANDATORY))); | ||
| 133 | } | 205 | } |
| 134 | 206 | ||
| 135 | - @Test(expected = IllegalArgumentException.class) | 207 | + @Test |
| 136 | - public void badIpAddress() { | 208 | + public void isConnectPoint() { |
| 137 | - assertTrue("is not proper ip", cfg.isIpAddress(TEXT, MANDATORY)); | 209 | + assertTrue("is not proper connectPoint", cfg.isConnectPoint(CONNECT_POINT, MANDATORY)); |
| 210 | + assertTrue("is not proper connectPoint", cfg.isConnectPoint(CONNECT_POINT, OPTIONAL)); | ||
| 211 | + assertTrue("is not proper connectPoint", cfg.isConnectPoint("none", OPTIONAL)); | ||
| 212 | + assertTrue("did not detect missing connectPoint", | ||
| 213 | + expectInvalidField(() -> cfg.isConnectPoint("none", MANDATORY))); | ||
| 214 | + assertTrue("did not detect bad connectPoint", | ||
| 215 | + expectInvalidField(() -> cfg.isConnectPoint(BAD_CONNECT_POINT, MANDATORY))); | ||
| 138 | } | 216 | } |
| 139 | 217 | ||
| 140 | @Test | 218 | @Test |
| ... | @@ -142,16 +220,28 @@ public class ConfigTest { | ... | @@ -142,16 +220,28 @@ public class ConfigTest { |
| 142 | assertTrue("is not proper transport port", cfg.isTpPort(TP_PORT, MANDATORY)); | 220 | assertTrue("is not proper transport port", cfg.isTpPort(TP_PORT, MANDATORY)); |
| 143 | assertTrue("is not proper transport port", cfg.isTpPort(TP_PORT, OPTIONAL)); | 221 | assertTrue("is not proper transport port", cfg.isTpPort(TP_PORT, OPTIONAL)); |
| 144 | assertTrue("is not proper transport port", cfg.isTpPort("none", OPTIONAL)); | 222 | assertTrue("is not proper transport port", cfg.isTpPort("none", OPTIONAL)); |
| 145 | - assertFalse("did not detect missing field", cfg.isTpPort("none", MANDATORY)); | 223 | + assertTrue("did not detect missing field", |
| 224 | + expectInvalidField(() -> cfg.isTpPort("none", MANDATORY))); | ||
| 225 | + assertTrue("is not proper transport port", | ||
| 226 | + expectInvalidField(() -> cfg.isTpPort(BAD_TP_PORT, MANDATORY))); | ||
| 146 | } | 227 | } |
| 147 | 228 | ||
| 148 | - @Test(expected = IllegalArgumentException.class) | 229 | + /** |
| 149 | - public void badTpPort() { | 230 | + * Expects an InvalidFieldException to be thrown when the given runnable is |
| 150 | - assertTrue("is not proper transport port", cfg.isTpPort(BAD_TP_PORT, MANDATORY)); | 231 | + * run. |
| 232 | + * | ||
| 233 | + * @param runnable runnable to run | ||
| 234 | + * @return true if an InvalidFieldException was thrown, otherwise false | ||
| 235 | + */ | ||
| 236 | + private boolean expectInvalidField(Runnable runnable) { | ||
| 237 | + try { | ||
| 238 | + runnable.run(); | ||
| 239 | + return false; | ||
| 240 | + } catch (InvalidFieldException e) { | ||
| 241 | + return true; | ||
| 242 | + } | ||
| 151 | } | 243 | } |
| 152 | 244 | ||
| 153 | - // TODO: Add tests for other helper methods | ||
| 154 | - | ||
| 155 | private class TestConfig extends Config<String> { | 245 | private class TestConfig extends Config<String> { |
| 156 | } | 246 | } |
| 157 | 247 | ||
| ... | @@ -160,4 +250,4 @@ public class ConfigTest { | ... | @@ -160,4 +250,4 @@ public class ConfigTest { |
| 160 | public void onApply(Config config) { | 250 | public void onApply(Config config) { |
| 161 | } | 251 | } |
| 162 | } | 252 | } |
| 163 | -} | ||
| ... | \ No newline at end of file | ... | \ No newline at end of file |
| 253 | +} | ... | ... |
| ... | @@ -40,6 +40,7 @@ import org.onlab.util.Tools; | ... | @@ -40,6 +40,7 @@ import org.onlab.util.Tools; |
| 40 | import org.onosproject.net.config.Config; | 40 | import org.onosproject.net.config.Config; |
| 41 | import org.onosproject.net.config.ConfigApplyDelegate; | 41 | import org.onosproject.net.config.ConfigApplyDelegate; |
| 42 | import org.onosproject.net.config.ConfigFactory; | 42 | import org.onosproject.net.config.ConfigFactory; |
| 43 | +import org.onosproject.net.config.InvalidConfigException; | ||
| 43 | import org.onosproject.net.config.NetworkConfigEvent; | 44 | import org.onosproject.net.config.NetworkConfigEvent; |
| 44 | import org.onosproject.net.config.NetworkConfigStore; | 45 | import org.onosproject.net.config.NetworkConfigStore; |
| 45 | import org.onosproject.net.config.NetworkConfigStoreDelegate; | 46 | import org.onosproject.net.config.NetworkConfigStoreDelegate; |
| ... | @@ -236,7 +237,17 @@ public class DistributedNetworkConfigStore | ... | @@ -236,7 +237,17 @@ public class DistributedNetworkConfigStore |
| 236 | public <S, C extends Config<S>> C applyConfig(S subject, Class<C> configClass, JsonNode json) { | 237 | public <S, C extends Config<S>> C applyConfig(S subject, Class<C> configClass, JsonNode json) { |
| 237 | // Create the configuration and validate it. | 238 | // Create the configuration and validate it. |
| 238 | C config = createConfig(subject, configClass, json); | 239 | C config = createConfig(subject, configClass, json); |
| 239 | - checkArgument(config.isValid(), INVALID_CONFIG_JSON); | 240 | + |
| 241 | + try { | ||
| 242 | + checkArgument(config.isValid(), INVALID_CONFIG_JSON); | ||
| 243 | + } catch (RuntimeException e) { | ||
| 244 | + ConfigFactory<S, C> configFactory = getConfigFactory(configClass); | ||
| 245 | + String subjectKey = configFactory.subjectFactory().subjectClassKey(); | ||
| 246 | + String subjectString = configFactory.subjectFactory().subjectKey(config.subject()); | ||
| 247 | + String configKey = config.key(); | ||
| 248 | + | ||
| 249 | + throw new InvalidConfigException(subjectKey, subjectString, configKey, e); | ||
| 250 | + } | ||
| 240 | 251 | ||
| 241 | // Insert the validated configuration and get it back. | 252 | // Insert the validated configuration and get it back. |
| 242 | Versioned<JsonNode> versioned = configs.putAndGet(key(subject, configClass), json); | 253 | Versioned<JsonNode> versioned = configs.putAndGet(key(subject, configClass), json); | ... | ... |
| 1 | +/* | ||
| 2 | + * Copyright 2016-present Open Networking Laboratory | ||
| 3 | + * | ||
| 4 | + * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| 5 | + * you may not use this file except in compliance with the License. | ||
| 6 | + * You may obtain a copy of the License at | ||
| 7 | + * | ||
| 8 | + * http://www.apache.org/licenses/LICENSE-2.0 | ||
| 9 | + * | ||
| 10 | + * Unless required by applicable law or agreed to in writing, software | ||
| 11 | + * distributed under the License is distributed on an "AS IS" BASIS, | ||
| 12 | + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| 13 | + * See the License for the specific language governing permissions and | ||
| 14 | + * limitations under the License. | ||
| 15 | + */ | ||
| 16 | + | ||
| 17 | +package org.onosproject.rest.exceptions; | ||
| 18 | + | ||
| 19 | +import com.fasterxml.jackson.databind.ObjectMapper; | ||
| 20 | +import com.fasterxml.jackson.databind.node.ObjectNode; | ||
| 21 | +import org.onlab.rest.exceptions.AbstractMapper; | ||
| 22 | +import org.onosproject.net.config.InvalidConfigException; | ||
| 23 | +import org.onosproject.net.config.InvalidFieldException; | ||
| 24 | + | ||
| 25 | +import javax.ws.rs.core.Response; | ||
| 26 | + | ||
| 27 | +/** | ||
| 28 | + * Maps InvalidConfigException to JSON output. | ||
| 29 | + */ | ||
| 30 | +public class InvalidConfigExceptionMapper extends AbstractMapper<InvalidConfigException> { | ||
| 31 | + | ||
| 32 | + @Override | ||
| 33 | + protected Response.Status responseStatus() { | ||
| 34 | + return Response.Status.BAD_REQUEST; | ||
| 35 | + } | ||
| 36 | + | ||
| 37 | + @Override | ||
| 38 | + protected Response.ResponseBuilder response(Response.Status status, Throwable exception) { | ||
| 39 | + error = exception; | ||
| 40 | + | ||
| 41 | + InvalidConfigException ex = (InvalidConfigException) exception; | ||
| 42 | + | ||
| 43 | + ObjectMapper mapper = new ObjectMapper(); | ||
| 44 | + String message = messageFrom(exception); | ||
| 45 | + ObjectNode result = mapper.createObjectNode() | ||
| 46 | + .put("code", status.getStatusCode()) | ||
| 47 | + .put("message", message) | ||
| 48 | + .put("subjectKey", ex.subjectKey()) | ||
| 49 | + .put("subject", ex.subject()) | ||
| 50 | + .put("configKey", ex.configKey()); | ||
| 51 | + | ||
| 52 | + if (ex.getCause() instanceof InvalidFieldException) { | ||
| 53 | + InvalidFieldException fieldException = (InvalidFieldException) ex.getCause(); | ||
| 54 | + result.put("field", fieldException.field()) | ||
| 55 | + .put("reason", fieldException.reason()); | ||
| 56 | + } | ||
| 57 | + | ||
| 58 | + return Response.status(status).entity(result.toString()); | ||
| 59 | + } | ||
| 60 | +} |
| 1 | +/* | ||
| 2 | + * Copyright 2016-present Open Networking Laboratory | ||
| 3 | + * | ||
| 4 | + * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| 5 | + * you may not use this file except in compliance with the License. | ||
| 6 | + * You may obtain a copy of the License at | ||
| 7 | + * | ||
| 8 | + * http://www.apache.org/licenses/LICENSE-2.0 | ||
| 9 | + * | ||
| 10 | + * Unless required by applicable law or agreed to in writing, software | ||
| 11 | + * distributed under the License is distributed on an "AS IS" BASIS, | ||
| 12 | + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| 13 | + * See the License for the specific language governing permissions and | ||
| 14 | + * limitations under the License. | ||
| 15 | + */ | ||
| 16 | + | ||
| 17 | +/** | ||
| 18 | + * REST exceptions. | ||
| 19 | + */ | ||
| 20 | +package org.onosproject.rest.exceptions; |
| ... | @@ -17,6 +17,7 @@ | ... | @@ -17,6 +17,7 @@ |
| 17 | package org.onosproject.rest.resources; | 17 | package org.onosproject.rest.resources; |
| 18 | 18 | ||
| 19 | import org.onlab.rest.AbstractWebApplication; | 19 | import org.onlab.rest.AbstractWebApplication; |
| 20 | +import org.onosproject.rest.exceptions.InvalidConfigExceptionMapper; | ||
| 20 | 21 | ||
| 21 | import java.util.Set; | 22 | import java.util.Set; |
| 22 | 23 | ||
| ... | @@ -49,7 +50,8 @@ public class CoreWebApplication extends AbstractWebApplication { | ... | @@ -49,7 +50,8 @@ public class CoreWebApplication extends AbstractWebApplication { |
| 49 | RegionsWebResource.class, | 50 | RegionsWebResource.class, |
| 50 | TenantWebResource.class, | 51 | TenantWebResource.class, |
| 51 | VirtualNetworkWebResource.class, | 52 | VirtualNetworkWebResource.class, |
| 52 | - MastershipWebResource.class | 53 | + MastershipWebResource.class, |
| 54 | + InvalidConfigExceptionMapper.class | ||
| 53 | ); | 55 | ); |
| 54 | } | 56 | } |
| 55 | } | 57 | } | ... | ... |
-
Please register or login to post a comment