You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2021/03/23 13:22:07 UTC
[brooklyn-server] 02/03: better detection of config constraint
violations
This is an automated email from the ASF dual-hosted git repository.
heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit ed825602a11b792d9f4363d166f35bf496c40c64
Author: Alex Heneveld <al...@cloudsoftcorp.com>
AuthorDate: Tue Mar 23 13:21:35 2021 +0000
better detection of config constraint violations
---
.../brooklyn/core/config/ConfigConstraints.java | 29 +++++++++++++---------
.../core/config/ConstraintViolationException.java | 2 +-
.../brooklyn/core/mgmt/DeployFailureTest.java | 5 ++--
3 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java b/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java
index e4d353f..2d92c45 100644
--- a/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java
+++ b/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java
@@ -158,19 +158,24 @@ public abstract class ConfigConstraints<T> {
Iterable<ConfigKey<?>> configKeys = getConfigKeys();
LOG.trace("Checking config keys on {}: {}", getSource(), configKeys);
for (ConfigKey<?> configKey : configKeys) {
- Maybe<?> maybeValue = getValue(configKey);
- if (maybeValue.isPresent()) {
- // Cast is safe because the author of the constraint on the config key had to
- // keep its type to Predicate<? super T>, where T is ConfigKey<T>.
- ReferenceWithError<?> validation = validateValue((ConfigKey<Object>) configKey, maybeValue.get());
- if (validation.hasError()) {
- violating.put(configKey, validation.getError());
+ try {
+ Maybe<?> maybeValue = getValue(configKey);
+ if (maybeValue.isPresent()) {
+ // Cast is safe because the author of the constraint on the config key had to
+ // keep its type to Predicate<? super T>, where T is ConfigKey<T>.
+ ReferenceWithError<?> validation = validateValue((ConfigKey<Object>) configKey, maybeValue.get());
+ if (validation.hasError()) {
+ violating.put(configKey, validation.getError());
+ }
+ } else {
+ // absent means did not resolve in time or not coercible;
+ // code will return `Maybe.of(null)` if it is unset,
+ // and coercion errors are handled when the value is _set_ or _needed_
+ // (this allows us to deal with edge cases where we can't *immediately* coerce)
}
- } else {
- // absent means did not resolve in time or not coercible;
- // code will return `Maybe.of(null)` if it is unset,
- // and coercion errors are handled when the value is _set_ or _needed_
- // (this allows us to deal with edge cases where we can't *immediately* coerce)
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ violating.put(configKey, e);
}
}
return violating;
diff --git a/core/src/main/java/org/apache/brooklyn/core/config/ConstraintViolationException.java b/core/src/main/java/org/apache/brooklyn/core/config/ConstraintViolationException.java
index 5ad490a..d99acc5 100644
--- a/core/src/main/java/org/apache/brooklyn/core/config/ConstraintViolationException.java
+++ b/core/src/main/java/org/apache/brooklyn/core/config/ConstraintViolationException.java
@@ -168,7 +168,7 @@ public class ConstraintViolationException extends UserFacingException {
Set<ConfigKey<?>> keys = MutableSet.copyOf(getConfigKeys(cves)).putAll(violations.keySet());
if (keys.size() > 1) {
- message.append("Invalid values for " + keys.stream().map(ConfigKey::getName) + ": ");
+ message.append("Invalid values for " + keys.stream().map(ConfigKey::getName).collect(Collectors.toSet()) + ": ");
}
if (!violations.isEmpty()) {
message.append(violations.values().stream().map(Throwable::toString).collect(Collectors.toSet()));
diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/DeployFailureTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/DeployFailureTest.java
index b9b5bb0..281b148 100644
--- a/core/src/test/java/org/apache/brooklyn/core/mgmt/DeployFailureTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/DeployFailureTest.java
@@ -106,9 +106,8 @@ public class DeployFailureTest extends BrooklynAppUnitTestSupport {
.child(EntitySpec.create(TestEntity.class)
.impl(TestEntityFailingGetParentImpl.class)));
Asserts.shouldHaveFailedPreviously();
- } catch (ClassCastException e) {
- // TODO Should give nicer exception
- Asserts.expectedFailureContains(e, "cannot be cast", "WrongParentEntity");
+ } catch (ConstraintViolationException e) {
+ Asserts.expectedFailureContains(e, "cannot be cast", "WrongParentEntity", TestEntityFailingGetParentImpl.class.getSimpleName());
}
// This should continue to work, after the failed entity-deploy above