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
The following commit(s) were added to refs/heads/master by this push:
new 884a63dc71 Skip property validation against the constraint if property
value is null
new 61e6c76f3e Merge remote-tracking branch
'algairim/improvements/constraints'
884a63dc71 is described below
commit 884a63dc712c25b14a00fa9461df6bf42090939f
Author: Mykola Mandra <[email protected]>
AuthorDate: Fri Aug 12 17:33:38 2022 +0100
Skip property validation against the constraint if property value is null
This skips the validation of the property if no constratint is present
that has 'required' word or 'Predicates.notNull()', when the property
value is null.
Signed-off-by: Mykola Mandra <[email protected]>
---
.../camp/brooklyn/ConfigParametersYamlTest.java | 27 ++++-----
.../brooklyn/core/config/ConfigConstraints.java | 24 ++++----
.../core/config/ConfigKeyConstraintTest.java | 64 +++++++++++++++++-----
3 files changed, 79 insertions(+), 36 deletions(-)
diff --git
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java
index 94e7bd4a36..592684027f 100644
---
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java
+++
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java
@@ -19,7 +19,6 @@
package org.apache.brooklyn.camp.brooklyn;
import com.google.common.base.Joiner;
-import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableList;
@@ -27,9 +26,6 @@ import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.reflect.TypeToken;
-import java.util.*;
-import java.util.function.BiConsumer;
-import java.util.stream.Collectors;
import org.apache.brooklyn.api.catalog.CatalogConfig;
import org.apache.brooklyn.api.entity.Entity;
import org.apache.brooklyn.api.entity.EntitySpec;
@@ -49,6 +45,7 @@ import
org.apache.brooklyn.core.config.ConstraintViolationException;
import org.apache.brooklyn.core.entity.AbstractEntity;
import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
import org.apache.brooklyn.core.entity.Dumper;
+import org.apache.brooklyn.core.entity.Entities;
import org.apache.brooklyn.core.location.PortRanges;
import org.apache.brooklyn.core.resolve.jackson.BrooklynJacksonType;
import
org.apache.brooklyn.core.resolve.jackson.BrooklynRegisteredTypeJacksonSerializationTest.SampleBean;
@@ -72,13 +69,17 @@ import org.apache.brooklyn.util.time.Timestamp;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.testng.Assert;
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertNotNull;
-import static org.testng.Assert.assertTrue;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
+import java.util.*;
+import java.util.function.BiConsumer;
+import java.util.stream.Collectors;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static org.testng.Assert.*;
+
public class ConfigParametersYamlTest extends AbstractYamlRebindTest {
private static final Logger LOG =
LoggerFactory.getLogger(ConfigParametersYamlTest.class);
@@ -1107,10 +1108,10 @@ public class ConfigParametersYamlTest extends
AbstractYamlRebindTest {
" testRequired: myprefix-myVal");
try {
- createStartWaitAndLogApplication(yamlNoVal);
- Asserts.shouldHaveFailedPreviously();
+ Entity app = createStartWaitAndLogApplication(yamlNoVal);
+ Entities.destroy(app);
} catch (ConstraintViolationException e) {
- Asserts.expectedFailureContains(e, "matchesRegex"); // success
+ Asserts.fail("validation of an absent value must be skipped for
matchesRegex");
}
try {
@@ -1205,10 +1206,10 @@ public class ConfigParametersYamlTest extends
AbstractYamlRebindTest {
" testRequired: myprefix-myVal");
try {
- createStartWaitAndLogApplication(yamlNoVal);
- Asserts.shouldHaveFailedPreviously();
+ Entity app = createStartWaitAndLogApplication(yamlNoVal);
+ Entities.destroy(app);
} catch (ConstraintViolationException e) {
- Asserts.expectedFailureContains(e, "Error configuring",
"PredicateRegexPojo(myprefix.*)"); // success
+ Asserts.fail("validation of an absent value must be skipped for
PredicateRegexPojo(myprefix.*)");
}
try {
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 5a87311706..b76d327119 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
@@ -19,11 +19,10 @@
package org.apache.brooklyn.core.config;
-import java.util.*;
-import java.util.concurrent.Future;
-import java.util.stream.Collectors;
-
-import javax.annotation.Nullable;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
import org.apache.brooklyn.api.entity.Entity;
import org.apache.brooklyn.api.location.Location;
import org.apache.brooklyn.api.mgmt.ExecutionContext;
@@ -47,11 +46,10 @@ import
org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.google.common.base.Predicate;
-import com.google.common.base.Predicates;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
+import javax.annotation.Nullable;
+import java.util.*;
+import java.util.concurrent.Future;
+import java.util.stream.Collectors;
/**
* Checks configuration constraints on entities and their adjuncts.
@@ -226,6 +224,12 @@ public abstract class ConfigConstraints<T> {
if (getSource() instanceof BrooklynObject && po instanceof
BrooklynObjectPredicate) {
valid = BrooklynObjectPredicate.class.cast(po).apply(value,
(BrooklynObject) getSource());
} else {
+ if (Objects.isNull(value)
+ && !po.toString().contains("required")
+ && !po.toString().contains("Predicates.notNull()")) {
+ // Skip validation if config key is optional and not
supplied.
+ return ReferenceWithError.newInstanceWithoutError(null);
+ }
valid = po.apply(value);
}
if (!valid) {
diff --git
a/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyConstraintTest.java
b/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyConstraintTest.java
index 10f03b1d2e..1db71503ad 100644
---
a/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyConstraintTest.java
+++
b/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyConstraintTest.java
@@ -19,13 +19,11 @@
package org.apache.brooklyn.core.config;
-import static org.testng.Assert.assertFalse;
-
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.Callable;
-import java.util.function.Consumer;
-
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Range;
import org.apache.brooklyn.api.entity.Entity;
import org.apache.brooklyn.api.entity.EntitySpec;
import org.apache.brooklyn.api.entity.ImplementedBy;
@@ -51,6 +49,7 @@ import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.core.task.Tasks;
import org.apache.brooklyn.util.javalang.JavaClassNames;
+import org.apache.brooklyn.util.math.MathPredicates;
import org.apache.brooklyn.util.net.Networking;
import org.apache.brooklyn.util.time.Duration;
import org.apache.brooklyn.util.time.Time;
@@ -59,13 +58,13 @@ import org.slf4j.LoggerFactory;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
-import com.google.common.base.Predicate;
-import com.google.common.base.Predicates;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Range;
-
import javax.annotation.Nonnull;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Callable;
+import java.util.function.Consumer;
+
+import static org.testng.Assert.assertFalse;
public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport {
@@ -73,6 +72,28 @@ public class ConfigKeyConstraintTest extends
BrooklynAppUnitTestSupport {
// ----------- Setup
-----------------------------------------------------------------------------------------------
+ @ImplementedBy(EntityWithNonRequiredConstraintImpl.class)
+ public static interface EntityWithNonRequiredConstraint extends TestEntity
{
+ ConfigKey<Number> OPTIONAL_CONFIG = ConfigKeys.builder(Number.class)
+ .name("test.conf.non-required.without-default")
+ .description("Configuration key that is optional")
+ .constraint(MathPredicates.greaterThan(2))
+ .build();
+ }
+ public static class EntityWithNonRequiredConstraintImpl extends
TestEntityImpl implements EntityWithNonRequiredConstraint {
+ }
+
+ @ImplementedBy(EntityWithRequiredConstraintImpl.class)
+ public static interface EntityWithRequiredConstraint extends TestEntity {
+ ConfigKey<Object> REQUIRED_CONFIG = ConfigKeys.builder(Object.class)
+ .name("test.conf.required.without-default")
+ .description("Configuration key that is required")
+ .constraint(ConfigConstraints.required())
+ .build();
+ }
+ public static class EntityWithRequiredConstraintImpl extends
TestEntityImpl implements EntityWithRequiredConstraint {
+ }
+
@ImplementedBy(EntityWithNonNullConstraintImpl.class)
public static interface EntityWithNonNullConstraint extends TestEntity {
ConfigKey<Object> NON_NULL_CONFIG = ConfigKeys.builder(Object.class)
@@ -156,6 +177,16 @@ public class ConfigKeyConstraintTest extends
BrooklynAppUnitTestSupport {
// ----------- Tests
-----------------------------------------------------------------------------------------------
+ @Test
+ public void testExceptionWhenEntityRequiredConfig() {
+ try {
+
app.createAndManageChild(EntitySpec.create(EntityWithRequiredConstraint.class));
+ Asserts.shouldHaveFailedPreviously("Expected exception when
managing entity with missing config");
+ } catch (Exception e) {
+ Asserts.expectedFailureOfType(e,
ConstraintViolationException.class);
+ }
+ }
+
@Test
public void testExceptionWhenEntityHasNullConfig() {
try {
@@ -170,6 +201,13 @@ public class ConfigKeyConstraintTest extends
BrooklynAppUnitTestSupport {
public void testNoExceptionWhenEntityHasValueForRequiredConfig() {
app.createAndManageChild(EntitySpec.create(EntityWithNonNullConstraint.class)
.configure(EntityWithNonNullConstraint.NON_NULL_CONFIG, new
Object()));
+
app.createAndManageChild(EntitySpec.create(EntityWithRequiredConstraint.class)
+ .configure(EntityWithRequiredConstraint.REQUIRED_CONFIG, new
Object()));
+ }
+
+ @Test
+ public void testNoExceptionWhenEntityHasNoValueForOptionalConfig() {
+
app.createAndManageChild(EntitySpec.create(EntityWithNonRequiredConstraint.class));
}
@Test
