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 2022/08/30 10:28:50 UTC
[brooklyn-server] branch master updated: Skip property validation against the constraint if property value is null
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 <my...@cloudsoft.io>
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 <my...@cloudsoft.io>
---
.../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