You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by jerrypeng <gi...@git.apache.org> on 2015/10/07 01:07:00 UTC
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
GitHub user jerrypeng opened a pull request:
https://github.com/apache/storm/pull/785
[STORM-1084] - Improve Storm config validation process to use java annotations instead of *_SCHEMA format
So currently we specify validators:
public static final String STORM_MESSAGING_NETTY_MIN_SLEEP_MS = "storm.messaging.netty.min_wait_ms";
public static final Object STORM_MESSAGING_NETTY_MIN_SLEEP_MS_SCHEMA = ConfigValidation.IntegerValidator;
A better way to do this is using annotations. Something like:
@IntegerValidator
public static final String STORM_MESSAGING_NETTY_MIN_SLEEP_MS = "storm.messaging.netty.min_wait_ms";
Do this has many advantages. For one you can stack multiple annotations:
@IntegerValidator
@NotNull
public static final String STORM_MESSAGING_NETTY_MIN_SLEEP_MS = "storm.messaging.netty.min_wait_ms";
And we don't have to write another validator for strings that cannot be null
And we can pass parameters into the annotations:
@PositiveIntegerValidator(notNull=true)
public static final String DRPC_REQUEST_TIMEOUT_SECS = "drpc.request.timeout.secs";
instead of having to write another validator: ConfigValidation.NotNullPosIntegerValidator for checking for not null
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jerrypeng/storm STORM-1084
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/785.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #785
----
commit cfc11637132320b18208bb463d21f33665faf2c1
Author: Boyang Jerry Peng <je...@yahoo-inc.com>
Date: 2015-10-06T23:01:25Z
[STORM-1084] - Improve Storm config validation process to use java annotations instead of *_SCHEMA format
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-148098193
@d2r upmerged and added functionality. Ready for review
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-147760999
It seems ConfigValidation was changed on master since the merge base for this branch. Need to check.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r41886199
--- Diff: storm-core/test/jvm/backtype/storm/TestConfigValidate.java ---
@@ -0,0 +1,137 @@
+package backtype.storm;
+
+import backtype.storm.utils.Utils;
+import org.junit.Test;
+import org.junit.Assert;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import backtype.storm.validation.ConfigValidationAnnotations;
+import backtype.storm.validation.ConfigValidation;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.Map;
+
+/**
+ * Created by jerrypeng on 10/1/15.
+ */
--- End diff --
remove?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42030910
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements");
+ }
+ objectSet.add(o);
+ }
+ }
+ }
+
+ /**
+ * Validates a String or a list of Strings
+ */
+ public static class StringOrStringListValidator extends Validator {
+
+ private ConfigValidationUtils.FieldValidator fv = ConfigValidationUtils.listFv(String.class, false);
+
+ @Override
+ public void validateField(String name, Object o) {
+
+ if (o == null) {
+ return;
+ }
+ if (o instanceof String) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ try {
+ isIterable.validateField(name, Iterable.class, o);
+ } catch (Exception ex) {
+ }
+ this.fv.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates Kryo Registration
+ */
+ public static class KryoRegValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Iterable) {
+ for (Object e : (Iterable) o) {
+ if (e instanceof Map) {
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) e).entrySet()) {
+ if (!(entry.getKey() instanceof String) ||
+ !(entry.getValue() instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ } else if (!(e instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ return;
+ }
+ throw new IllegalArgumentException(
+ "Field " + name + " must be an Iterable containing only Strings or Maps of Strings");
+ }
+ }
+
+ /**
+ * Validates if a number is a power of 2
+ */
+ public static class PowerOf2Validator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ // Test whether the integer is a power of 2.
+ if (i > 0 && (i & (i - 1)) == 0) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be a power of 2.");
+ }
+ }
+
+ /**
+ * Validates each entry in a list
+ */
+ public static class ListEntryTypeValidator extends TypeValidator {
+
+ @Override
+ public void validateField(String name, Class type, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.listFv(type, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates entry in the a list with a list of custom validator
--- End diff --
will fix
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42028053
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements");
+ }
+ objectSet.add(o);
+ }
+ }
+ }
+
+ /**
+ * Validates a String or a list of Strings
+ */
+ public static class StringOrStringListValidator extends Validator {
+
+ private ConfigValidationUtils.FieldValidator fv = ConfigValidationUtils.listFv(String.class, false);
+
+ @Override
+ public void validateField(String name, Object o) {
+
+ if (o == null) {
+ return;
+ }
+ if (o instanceof String) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ try {
+ isIterable.validateField(name, Iterable.class, o);
+ } catch (Exception ex) {
+ }
+ this.fv.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates Kryo Registration
+ */
+ public static class KryoRegValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Iterable) {
+ for (Object e : (Iterable) o) {
+ if (e instanceof Map) {
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) e).entrySet()) {
+ if (!(entry.getKey() instanceof String) ||
+ !(entry.getValue() instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ } else if (!(e instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ return;
+ }
+ throw new IllegalArgumentException(
+ "Field " + name + " must be an Iterable containing only Strings or Maps of Strings");
+ }
+ }
+
+ /**
+ * Validates if a number is a power of 2
+ */
+ public static class PowerOf2Validator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ // Test whether the integer is a power of 2.
+ if (i > 0 && (i & (i - 1)) == 0) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be a power of 2.");
+ }
+ }
+
+ /**
+ * Validates each entry in a list
+ */
+ public static class ListEntryTypeValidator extends TypeValidator {
+
+ @Override
+ public void validateField(String name, Class type, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.listFv(type, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates entry in the a list with a list of custom validator
+ */
+ public static class ListEntryCustomValidator {
+
+ public void validateField(String name, Class[] validators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, o);
+ for (Object entry : (Iterable) o) {
+ for (Class validator : validators) {
+ Object v = validator.newInstance();
+ if (v instanceof Validator) {
+ ((Validator) v).validateField(name + " list entry", entry);
+ } else {
+ LOG.warn("validator: {} cannot be used in ListEntryCustomValidator. Individual entry validators must a instance of Validator class", validator.getName());
+ }
+ }
+ }
+
+ }
+ }
+
+ /**
+ * validates each key and value in a map of a certain type
+ */
+ public static class MapEntryTypeValidator {
+
+ public void validateField(String name, Class keyType, Class valueType, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(keyType, valueType, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ public static class MapEntryCustomValidator {
+
+ public void validateField(String name, Class[] keyValidators, Class[] valueValidators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if Map
+ SimpleTypeValidator isMap = new SimpleTypeValidator();
+ isMap.validateField(name, Map.class, o);
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) o).entrySet()) {
+ for (Class kv : keyValidators) {
+ Object keyValidator = kv.newInstance();
+ if (keyValidator instanceof Validator) {
+ ((Validator) keyValidator).validateField(name + " Map key", entry.getKey());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate keys. Individual entry validators must a instance of Validator class", kv.getName());
+ }
+ }
+ for (Class vv : valueValidators) {
+ Object valueValidator = vv.newInstance();
+ if (valueValidator instanceof Validator) {
+ ((Validator) valueValidator).validateField(name + " Map value", entry.getValue());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate values. Individual entry validators must a instance of Validator class", vv.getName());
+ }
+ }
+ }
+ }
+ }
+
+ /**
+ * Validates a positive number
+ */
+ public static class PositiveNumberValidator extends Validator{
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateField(name, false, o);
+ }
+
+ public void validateField(String name, boolean includeZero, Object o) {
--- End diff --
will fix
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42148487
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements");
+ }
+ objectSet.add(o);
+ }
+ }
+ }
+
+ /**
+ * Validates a String or a list of Strings
+ */
+ public static class StringOrStringListValidator extends Validator {
+
+ private ConfigValidationUtils.FieldValidator fv = ConfigValidationUtils.listFv(String.class, false);
+
+ @Override
+ public void validateField(String name, Object o) {
+
+ if (o == null) {
+ return;
+ }
+ if (o instanceof String) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ try {
+ isIterable.validateField(name, Iterable.class, o);
+ } catch (Exception ex) {
+ }
+ this.fv.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates Kryo Registration
+ */
+ public static class KryoRegValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Iterable) {
+ for (Object e : (Iterable) o) {
+ if (e instanceof Map) {
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) e).entrySet()) {
+ if (!(entry.getKey() instanceof String) ||
+ !(entry.getValue() instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ } else if (!(e instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ return;
+ }
+ throw new IllegalArgumentException(
+ "Field " + name + " must be an Iterable containing only Strings or Maps of Strings");
+ }
+ }
+
+ /**
+ * Validates if a number is a power of 2
+ */
+ public static class PowerOf2Validator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ // Test whether the integer is a power of 2.
+ if (i > 0 && (i & (i - 1)) == 0) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be a power of 2.");
+ }
+ }
+
+ /**
+ * Validates each entry in a list
+ */
+ public static class ListEntryTypeValidator extends TypeValidator {
+
+ @Override
+ public void validateField(String name, Class type, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.listFv(type, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates entry in the a list with a list of custom validator
+ */
+ public static class ListEntryCustomValidator {
+
+ public void validateField(String name, Class[] validators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, o);
+ for (Object entry : (Iterable) o) {
+ for (Class validator : validators) {
+ Object v = validator.newInstance();
+ if (v instanceof Validator) {
+ ((Validator) v).validateField(name + " list entry", entry);
+ } else {
+ LOG.warn("validator: {} cannot be used in ListEntryCustomValidator. Individual entry validators must a instance of Validator class", validator.getName());
+ }
+ }
+ }
+
+ }
+ }
+
+ /**
+ * validates each key and value in a map of a certain type
+ */
+ public static class MapEntryTypeValidator {
+
+ public void validateField(String name, Class keyType, Class valueType, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(keyType, valueType, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ public static class MapEntryCustomValidator {
+
+ public void validateField(String name, Class[] keyValidators, Class[] valueValidators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if Map
+ SimpleTypeValidator isMap = new SimpleTypeValidator();
+ isMap.validateField(name, Map.class, o);
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) o).entrySet()) {
+ for (Class kv : keyValidators) {
+ Object keyValidator = kv.newInstance();
+ if (keyValidator instanceof Validator) {
+ ((Validator) keyValidator).validateField(name + " Map key", entry.getKey());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate keys. Individual entry validators must a instance of Validator class", kv.getName());
+ }
+ }
+ for (Class vv : valueValidators) {
+ Object valueValidator = vv.newInstance();
+ if (valueValidator instanceof Validator) {
+ ((Validator) valueValidator).validateField(name + " Map value", entry.getValue());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate values. Individual entry validators must a instance of Validator class", vv.getName());
+ }
+ }
+ }
+ }
+ }
--- End diff --
Looks good.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-146272172
@revans2 your suggestion are great, but the parameters passed into java annotations have to declared to be a specific type thus we could not have:
@MapType(key=@StringType, value=@NumberType)
public static final ...;
but maybe something like:
@MapType(key=@TypeValidator(type = String.class), value=@TypeValidator(type = Number.class))
public static final ...;
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42032063
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidationAnnotations.java ---
@@ -0,0 +1,216 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.RetentionPolicy;
+
+/**
+ * Note: every annotation interface must have method validatorClass()
+ * For every annotation there must validator class to do the validation
+ * To add another annotation for config validation, add another annotation @interface class. Implement the corresponding
+ * validator logic in a class in ConfigValidation. Make sure validateField method in ConfigValidation knows how to use the validator
+ * and which method definition/parameters to pass in based on what fields are in the annotation.
+ */
+public class ConfigValidationAnnotations {
+ /**
+ * Field names for annotations
+ */
+
+ static final String VALIDATOR_CLASS = "validatorClass";
+ static final String TYPE = "type";
+ static final String ENTRY_VALIDATOR_CLASSES = "entryValidatorClasses";
+ static final String KEY_VALIDATOR_CLASSES = "keyValidatorClasses";
+ static final String VALUE_VALIDATOR_CLASSES = "valueValidatorClasses";
+ static final String KEY_TYPE = "keyType";
+ static final String VALUE_TYPE = "valueType";
+ static final String INCLUDE_ZERO = "includeZero";
+
+ /**
+ * Validators with fields: validatorClass and type
+ */
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target(ElementType.FIELD)
+ public @interface isType {
+ Class validatorClass() default ConfigValidation.SimpleTypeValidator.class;
+
+ Class type();
+ }
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target(ElementType.FIELD)
+ public @interface isStrings {
--- End diff --
Can we make this areStrings or isListOfStrings. isStrings is too close to isString for me, and it feels a bit grammatically suspect.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r41397489
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,359 @@
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Created by jerrypeng on 10/1/15.
--- End diff --
Please provide a description of the class instead of your name and date.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-146948299
@revans2 I don't think you solution will work either. I don't think we can get recursion and java annotations to work, since fields in annotations cannot default to a specific non-null type. To have annotations that contain fields of it self will result in a unending recursive definition:
ComplexValidator key() default @ComplexValidator(type=Object.class, key=@ComplexValidator(type=Object.class, key=@ComplexValidator(...), value=ComplexValidator(ype=Object.class, key=@ComplexValidator(...));
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42045972
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements");
+ }
+ objectSet.add(o);
+ }
+ }
+ }
+
+ /**
+ * Validates a String or a list of Strings
+ */
+ public static class StringOrStringListValidator extends Validator {
+
+ private ConfigValidationUtils.FieldValidator fv = ConfigValidationUtils.listFv(String.class, false);
+
+ @Override
+ public void validateField(String name, Object o) {
+
+ if (o == null) {
+ return;
+ }
+ if (o instanceof String) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ try {
+ isIterable.validateField(name, Iterable.class, o);
+ } catch (Exception ex) {
+ }
+ this.fv.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates Kryo Registration
+ */
+ public static class KryoRegValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Iterable) {
+ for (Object e : (Iterable) o) {
+ if (e instanceof Map) {
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) e).entrySet()) {
+ if (!(entry.getKey() instanceof String) ||
+ !(entry.getValue() instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ } else if (!(e instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ return;
+ }
+ throw new IllegalArgumentException(
+ "Field " + name + " must be an Iterable containing only Strings or Maps of Strings");
+ }
+ }
+
+ /**
+ * Validates if a number is a power of 2
+ */
+ public static class PowerOf2Validator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ // Test whether the integer is a power of 2.
+ if (i > 0 && (i & (i - 1)) == 0) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be a power of 2.");
+ }
+ }
+
+ /**
+ * Validates each entry in a list
+ */
+ public static class ListEntryTypeValidator extends TypeValidator {
+
+ @Override
+ public void validateField(String name, Class type, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.listFv(type, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates entry in the a list with a list of custom validator
+ */
+ public static class ListEntryCustomValidator {
+
+ public void validateField(String name, Class[] validators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, o);
+ for (Object entry : (Iterable) o) {
+ for (Class validator : validators) {
+ Object v = validator.newInstance();
+ if (v instanceof Validator) {
+ ((Validator) v).validateField(name + " list entry", entry);
+ } else {
+ LOG.warn("validator: {} cannot be used in ListEntryCustomValidator. Individual entry validators must a instance of Validator class", validator.getName());
+ }
+ }
+ }
+
+ }
+ }
+
+ /**
+ * validates each key and value in a map of a certain type
+ */
+ public static class MapEntryTypeValidator {
+
+ public void validateField(String name, Class keyType, Class valueType, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(keyType, valueType, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ public static class MapEntryCustomValidator {
+
+ public void validateField(String name, Class[] keyValidators, Class[] valueValidators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if Map
+ SimpleTypeValidator isMap = new SimpleTypeValidator();
+ isMap.validateField(name, Map.class, o);
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) o).entrySet()) {
+ for (Class kv : keyValidators) {
+ Object keyValidator = kv.newInstance();
+ if (keyValidator instanceof Validator) {
+ ((Validator) keyValidator).validateField(name + " Map key", entry.getKey());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate keys. Individual entry validators must a instance of Validator class", kv.getName());
+ }
+ }
+ for (Class vv : valueValidators) {
+ Object valueValidator = vv.newInstance();
+ if (valueValidator instanceof Validator) {
+ ((Validator) valueValidator).validateField(name + " Map value", entry.getValue());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate values. Individual entry validators must a instance of Validator class", vv.getName());
+ }
+ }
+ }
+ }
+ }
--- End diff --
will add
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42042650
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
--- End diff --
will fix
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-148462523
The test failure is an issue with maven trying to download something and getting a failure. I don't see how it is related.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-146250340
@jerrypeng I agree that we want a base set of annotations, and we can adjust what they do. I am just concerned about making that base set as flexible and complete as possible.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-148164422
This is nothing you did, but now that the validation is much more readable I am seeing a few things that should probably be fixed, but I am happy to do them myself on a follow on JIRA
`TOPOLOGY_ISOLATED_MACHINES` needs `@isPositiveNumber`
All of the 'ZMQ_` configs should be deprecated.
`TRANSACTIONAL_ZOOKEEPER_PORT`needs `@isPositiveNumber`
It would be great if we could restrict `TOPOLOGY_LOGGING_SENSITIVITY` to one of the allowed values "S0", "S1", "S2", "S3"
`TOPOLOGY_SHELLBOLT_MAX_PENDING` needs `@isPositiveNumber`
`TOPOLOGY_TRIDENT_BATCH_EMIT_INTERVAL_MILLIS` needs `@isPositiveNumber`
`TOPOLOGY_MAX_ERROR_REPORT_PER_INTERVAL` and `TOPOLOGY_ERROR_THROTTLE_INTERVAL_SECS` both seem to need `@isPositiveNumber`
`TOPOLOGY_TRANSFER_BUFFER_SIZE` needs to be `@isPowerOf2`
`TOPOLOGY_ENVIRONMENT` should be `@isMapEntryType(keyType = String.class, valueType = String.class)`.
`TOPOLOGY_SLEEP_SPOUT_WAIT_STRATEGY_TIME_MS` needs `@isPositiveNumber(includeZero = true)`
`TOPOLOGY_MAX_SPOUT_PENDING` needs `@isPositiveNumber`
`TOPOLOGY_MAX_TASK_PARALLELISM` needs `@isPositiveNumber`
`WORKER_METRICS` and `TOPOLOGY_WORKER_METRICS` should be `@isMapEntryType(keyType = String.class, valueType = String.class)`.
`TOPOLOGY_METRICS_CONSUMER_REGISTER` should have a custom validator (you might not have time to do it, so we might need a follow on JIRA for this). Something like
`@isListEntryCustom(entryValidatorClasses={MetricRegistryValidator.class})`
MetricRegistryValidator.class needs to check that it is a map, with a "class" key that points to a string, a "parallelism.hint" key that points to a positive non-null integer.
`TOPOLOGY_EVENTLOGGER_EXECUTORS` needs `@isPositiveNumber`
`TOPOLOGY_ACKER_EXECUTORS` needs `@isPositiveNumber`
`TOPOLOGY_TASKS` and `TOPOLOGY_WORKERS` need `@isPositiveNumber`
`TASK_CREDENTIALS_POLL_SECS` needs `@isPositiveNumber`
`TASK_REFRESH_POLL_SECS` `TASK_HEARTBEAT_FREQUENCY_SECS` `WORKER_HEARTBEAT_FREQUENCY_SECS` and `WORKER_RECEIVER_THREAD_COUNT` need `@isPositiveNumber`
`SUPERVISOR_MONITOR_FREQUENCY_SECS` and `SUPERVISOR_HEARTBEAT_FREQUENCY_SECS` need `@isPositiveNumber`
`SUPERVISOR_WORKER_SHUTDOWN_SLEEP_SECS` needs `@isPositiveNumber`
`DRPC_HTTP_FILTER_PARAMS` should be `@isMapEntryType(keyType = String.class, valueType = String.class)`.
`DRPC_INVOCATIONS_THREADS` and `DRPC_INVOCATIONS_PORT` need `@isPositiveNumber`
`DRPC_QUEUE_SIZE` `DRPC_MAX_BUFFER_SIZE` and `DRPC_WORKER_THREADS` need `@isPositiveNumber`
`DRPC_AUTHORIZER_ACL` needs to be a Map<String, Map<String, String or List<String>>>. This too probably needs a custom validator in a follow on JIRA.
`DRPC_PORT` needs `@isPositiveNumber`
`DRPC_HTTPS_PORT` and `DRPC_HTTP_PORT` need `@isPositiveNumber`
`UI_HTTPS_PORT` and `UI_HEADER_BUFFER_BYTES` need `@isPositiveNumber`
`UI_FILTER_PARAMS` should be `@isMapEntryType(keyType = String.class, valueType = String.class)`.
`LOGVIEWER_HTTPS_PORT` needs `@isPositiveNumber`
`LOGVIEWER_PORT` and `UI_PORT` need `@isPositiveNumber`
`NIMBUS_CREDENTIAL_RENEW_FREQ_SECS` needs `@isPositiveNumber`
`NIMBUS_IMPERSONATION_ACL` needs to be updated, because I don't think Map of string to map. It is more complex then that.
`NIMBUS_TASK_LAUNCH_SECS` `NIMBUS_SUPERVISOR_TIMEOUT_SECS` `NIMBUS_INBOX_JAR_EXPIRATION_SECS` `NIMBUS_CLEANUP_INBOX_FREQ_SECS` `NIMBUS_MONITOR_FREQ_SECS` and `NIMBUS_TASK_TIMEOUT_SECS` need `@isPositiveNumber`
`NIMBUS_THRIFT_MAX_BUFFER_SIZE` needs `@isPositiveNumber`
`NIMBUS_THRIFT_THREADS` and `NIMBUS_THRIFT_PORT` need `@isPositiveNumber`
I think there are more too so we probably need a follow on JIRA for this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42033497
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
--- End diff --
will change
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-146249930
Could we use nested annotations for the complex types?
https://blogs.oracle.com/toddfast/entry/creating_nested_complex_java_annotations
```
@MapType(key=@StringType, value=@NumberType)
public static final ...;
```
instead of
```
@MapOfStringToNumberValidator
```
We might be able to add in an `@Or` annotation so we could get some of the complex validators to be built up from smaller validators. So we don't need a once off Validator.
```
@StringOrStringListValidator
```
would become
```
@OR(@StringType, @ListType(@StringType))
```
To me this just gives users better options so they don't have to create custom validators and annotations whenever a new data structure is needed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-148468577
JDK8 CI test failed because connections to repo.maven.apache.org were reset, and so dependency resolution did not work. I ran all the tests on OSX myself and they passed for me.
I am +1, and thanks @jerrypeng for your patience during the review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42032775
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
--- End diff --
Realizing this validator name was copied from before, it would be really nice to clean up the name here. This really validates more than str->str->[str], but it also validates that the values of the nested map are lists of strings. This is so specialized that maybe we want to rename the validator and its annotation to something like `ImpersonationAclValidator` and `@isImpersonationAcl`.
The reason is, should someone else suppose it is as generic as the name describes, they may find out it is being too strict and rejecting config entries that the javadoc and name do not say should be invalid.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42027724
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements");
+ }
+ objectSet.add(o);
+ }
+ }
+ }
+
+ /**
+ * Validates a String or a list of Strings
+ */
+ public static class StringOrStringListValidator extends Validator {
+
+ private ConfigValidationUtils.FieldValidator fv = ConfigValidationUtils.listFv(String.class, false);
+
+ @Override
+ public void validateField(String name, Object o) {
+
+ if (o == null) {
+ return;
+ }
+ if (o instanceof String) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ try {
+ isIterable.validateField(name, Iterable.class, o);
+ } catch (Exception ex) {
+ }
+ this.fv.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates Kryo Registration
+ */
+ public static class KryoRegValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Iterable) {
+ for (Object e : (Iterable) o) {
+ if (e instanceof Map) {
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) e).entrySet()) {
+ if (!(entry.getKey() instanceof String) ||
+ !(entry.getValue() instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ } else if (!(e instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ return;
+ }
+ throw new IllegalArgumentException(
+ "Field " + name + " must be an Iterable containing only Strings or Maps of Strings");
+ }
+ }
+
+ /**
+ * Validates if a number is a power of 2
+ */
+ public static class PowerOf2Validator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ // Test whether the integer is a power of 2.
+ if (i > 0 && (i & (i - 1)) == 0) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be a power of 2.");
+ }
+ }
+
+ /**
+ * Validates each entry in a list
+ */
+ public static class ListEntryTypeValidator extends TypeValidator {
+
+ @Override
+ public void validateField(String name, Class type, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.listFv(type, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates entry in the a list with a list of custom validator
+ */
+ public static class ListEntryCustomValidator {
+
+ public void validateField(String name, Class[] validators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, o);
+ for (Object entry : (Iterable) o) {
+ for (Class validator : validators) {
+ Object v = validator.newInstance();
+ if (v instanceof Validator) {
+ ((Validator) v).validateField(name + " list entry", entry);
+ } else {
+ LOG.warn("validator: {} cannot be used in ListEntryCustomValidator. Individual entry validators must a instance of Validator class", validator.getName());
+ }
+ }
+ }
+
+ }
+ }
+
+ /**
+ * validates each key and value in a map of a certain type
+ */
+ public static class MapEntryTypeValidator {
+
+ public void validateField(String name, Class keyType, Class valueType, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(keyType, valueType, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ public static class MapEntryCustomValidator {
+
+ public void validateField(String name, Class[] keyValidators, Class[] valueValidators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if Map
+ SimpleTypeValidator isMap = new SimpleTypeValidator();
+ isMap.validateField(name, Map.class, o);
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) o).entrySet()) {
+ for (Class kv : keyValidators) {
+ Object keyValidator = kv.newInstance();
+ if (keyValidator instanceof Validator) {
+ ((Validator) keyValidator).validateField(name + " Map key", entry.getKey());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate keys. Individual entry validators must a instance of Validator class", kv.getName());
+ }
+ }
+ for (Class vv : valueValidators) {
+ Object valueValidator = vv.newInstance();
+ if (valueValidator instanceof Validator) {
+ ((Validator) valueValidator).validateField(name + " Map value", entry.getValue());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate values. Individual entry validators must a instance of Validator class", vv.getName());
+ }
+ }
+ }
+ }
+ }
+
+ /**
+ * Validates a positive number
+ */
+ public static class PositiveNumberValidator extends Validator{
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateField(name, false, o);
+ }
+
+ public void validateField(String name, boolean includeZero, Object o) {
--- End diff --
check indentation of method declaration
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-148442586
@revans2 Thanks! You are right that piece of code involving the try catch statement is useless. I have removed it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42029112
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements");
+ }
+ objectSet.add(o);
+ }
+ }
+ }
+
+ /**
+ * Validates a String or a list of Strings
+ */
+ public static class StringOrStringListValidator extends Validator {
+
+ private ConfigValidationUtils.FieldValidator fv = ConfigValidationUtils.listFv(String.class, false);
+
+ @Override
+ public void validateField(String name, Object o) {
+
+ if (o == null) {
+ return;
+ }
+ if (o instanceof String) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ try {
+ isIterable.validateField(name, Iterable.class, o);
+ } catch (Exception ex) {
+ }
+ this.fv.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates Kryo Registration
+ */
+ public static class KryoRegValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Iterable) {
+ for (Object e : (Iterable) o) {
+ if (e instanceof Map) {
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) e).entrySet()) {
+ if (!(entry.getKey() instanceof String) ||
+ !(entry.getValue() instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ } else if (!(e instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ return;
+ }
+ throw new IllegalArgumentException(
+ "Field " + name + " must be an Iterable containing only Strings or Maps of Strings");
+ }
+ }
+
+ /**
+ * Validates if a number is a power of 2
+ */
+ public static class PowerOf2Validator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ // Test whether the integer is a power of 2.
+ if (i > 0 && (i & (i - 1)) == 0) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be a power of 2.");
+ }
+ }
+
+ /**
+ * Validates each entry in a list
+ */
+ public static class ListEntryTypeValidator extends TypeValidator {
+
+ @Override
+ public void validateField(String name, Class type, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.listFv(type, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates entry in the a list with a list of custom validator
--- End diff --
Maybe this? `Validates each entry in a list against a list of custom Validators`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42042530
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
--- End diff --
"duplicates"
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42048224
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements");
+ }
+ objectSet.add(o);
+ }
+ }
+ }
+
+ /**
+ * Validates a String or a list of Strings
+ */
+ public static class StringOrStringListValidator extends Validator {
+
+ private ConfigValidationUtils.FieldValidator fv = ConfigValidationUtils.listFv(String.class, false);
+
+ @Override
+ public void validateField(String name, Object o) {
+
+ if (o == null) {
+ return;
+ }
+ if (o instanceof String) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ try {
+ isIterable.validateField(name, Iterable.class, o);
+ } catch (Exception ex) {
+ }
+ this.fv.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates Kryo Registration
+ */
+ public static class KryoRegValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Iterable) {
+ for (Object e : (Iterable) o) {
+ if (e instanceof Map) {
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) e).entrySet()) {
+ if (!(entry.getKey() instanceof String) ||
+ !(entry.getValue() instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ } else if (!(e instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ return;
+ }
+ throw new IllegalArgumentException(
+ "Field " + name + " must be an Iterable containing only Strings or Maps of Strings");
+ }
+ }
+
+ /**
+ * Validates if a number is a power of 2
+ */
+ public static class PowerOf2Validator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ // Test whether the integer is a power of 2.
+ if (i > 0 && (i & (i - 1)) == 0) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be a power of 2.");
+ }
+ }
+
+ /**
+ * Validates each entry in a list
+ */
+ public static class ListEntryTypeValidator extends TypeValidator {
+
+ @Override
+ public void validateField(String name, Class type, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.listFv(type, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates entry in the a list with a list of custom validator
+ */
+ public static class ListEntryCustomValidator {
+
+ public void validateField(String name, Class[] validators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, o);
+ for (Object entry : (Iterable) o) {
+ for (Class validator : validators) {
+ Object v = validator.newInstance();
+ if (v instanceof Validator) {
+ ((Validator) v).validateField(name + " list entry", entry);
+ } else {
+ LOG.warn("validator: {} cannot be used in ListEntryCustomValidator. Individual entry validators must a instance of Validator class", validator.getName());
+ }
+ }
+ }
+
+ }
+ }
+
+ /**
+ * validates each key and value in a map of a certain type
+ */
+ public static class MapEntryTypeValidator {
+
+ public void validateField(String name, Class keyType, Class valueType, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(keyType, valueType, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ public static class MapEntryCustomValidator {
+
+ public void validateField(String name, Class[] keyValidators, Class[] valueValidators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if Map
+ SimpleTypeValidator isMap = new SimpleTypeValidator();
+ isMap.validateField(name, Map.class, o);
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) o).entrySet()) {
+ for (Class kv : keyValidators) {
+ Object keyValidator = kv.newInstance();
+ if (keyValidator instanceof Validator) {
+ ((Validator) keyValidator).validateField(name + " Map key", entry.getKey());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate keys. Individual entry validators must a instance of Validator class", kv.getName());
+ }
+ }
+ for (Class vv : valueValidators) {
+ Object valueValidator = vv.newInstance();
+ if (valueValidator instanceof Validator) {
+ ((Validator) valueValidator).validateField(name + " Map value", entry.getValue());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate values. Individual entry validators must a instance of Validator class", vv.getName());
+ }
+ }
+ }
+ }
+ }
+
+ /**
+ * Validates a positive number
+ */
+ public static class PositiveNumberValidator extends Validator{
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateField(name, false, o);
+ }
+
+ public void validateField(String name, boolean includeZero, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Number) {
+ if(includeZero == true) {
+ if (((Number) o).doubleValue() >= 0.0) {
+ return;
+ }
+ } else {
+ if (((Number) o).doubleValue() > 0.0) {
+ return;
+ }
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be a Positive Number");
+ }
+ }
+
+ /**
+ * Methods for validating confs
+ */
+
+ /**
+ * Validates a field given field name as string
+ *
+ * @param fieldName provided as a string
+ * @param conf map of confs
+ */
+ public static void validateField(String fieldName, Map conf) throws NoSuchFieldException, IllegalAccessException, InstantiationException, NoSuchMethodException, InvocationTargetException {
+ validateField(fieldName, conf, CONFIG_CLASS);
+ }
--- End diff --
useful if a developer would like to validate a field in a conf map at any point. The developer can just call:
validateField(Config.EXAMPLE_CONF, confMap)
instead of trying to get the corresponding Field object first. Just make the code cleaner if someone else decides to validates some configs in the future
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42045623
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements");
+ }
+ objectSet.add(o);
+ }
+ }
+ }
+
+ /**
+ * Validates a String or a list of Strings
+ */
+ public static class StringOrStringListValidator extends Validator {
+
+ private ConfigValidationUtils.FieldValidator fv = ConfigValidationUtils.listFv(String.class, false);
+
+ @Override
+ public void validateField(String name, Object o) {
+
+ if (o == null) {
+ return;
+ }
+ if (o instanceof String) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ try {
+ isIterable.validateField(name, Iterable.class, o);
+ } catch (Exception ex) {
+ }
+ this.fv.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates Kryo Registration
+ */
+ public static class KryoRegValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Iterable) {
+ for (Object e : (Iterable) o) {
+ if (e instanceof Map) {
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) e).entrySet()) {
+ if (!(entry.getKey() instanceof String) ||
+ !(entry.getValue() instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ } else if (!(e instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ return;
+ }
+ throw new IllegalArgumentException(
+ "Field " + name + " must be an Iterable containing only Strings or Maps of Strings");
+ }
+ }
+
+ /**
+ * Validates if a number is a power of 2
+ */
+ public static class PowerOf2Validator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ // Test whether the integer is a power of 2.
+ if (i > 0 && (i & (i - 1)) == 0) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be a power of 2.");
+ }
+ }
+
+ /**
+ * Validates each entry in a list
+ */
+ public static class ListEntryTypeValidator extends TypeValidator {
+
+ @Override
+ public void validateField(String name, Class type, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.listFv(type, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates entry in the a list with a list of custom validator
+ */
+ public static class ListEntryCustomValidator {
+
+ public void validateField(String name, Class[] validators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, o);
+ for (Object entry : (Iterable) o) {
+ for (Class validator : validators) {
+ Object v = validator.newInstance();
+ if (v instanceof Validator) {
+ ((Validator) v).validateField(name + " list entry", entry);
+ } else {
+ LOG.warn("validator: {} cannot be used in ListEntryCustomValidator. Individual entry validators must a instance of Validator class", validator.getName());
+ }
+ }
+ }
+
+ }
+ }
+
+ /**
+ * validates each key and value in a map of a certain type
+ */
+ public static class MapEntryTypeValidator {
+
+ public void validateField(String name, Class keyType, Class valueType, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(keyType, valueType, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ public static class MapEntryCustomValidator {
+
+ public void validateField(String name, Class[] keyValidators, Class[] valueValidators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if Map
+ SimpleTypeValidator isMap = new SimpleTypeValidator();
+ isMap.validateField(name, Map.class, o);
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) o).entrySet()) {
+ for (Class kv : keyValidators) {
+ Object keyValidator = kv.newInstance();
+ if (keyValidator instanceof Validator) {
+ ((Validator) keyValidator).validateField(name + " Map key", entry.getKey());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate keys. Individual entry validators must a instance of Validator class", kv.getName());
+ }
+ }
+ for (Class vv : valueValidators) {
+ Object valueValidator = vv.newInstance();
+ if (valueValidator instanceof Validator) {
+ ((Validator) valueValidator).validateField(name + " Map value", entry.getValue());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate values. Individual entry validators must a instance of Validator class", vv.getName());
+ }
+ }
+ }
+ }
+ }
--- End diff --
It might be good to add a concise javadoc for this class as we have done for others; maybe something like `validates each key and each value against the respective arrays of validators`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-146246638
@jerrypeng
I keep going back and forth on how we want the annotations to work. Right now they are more or less a one to one translation of what we had before. Admittedly it is a lot cleaner than it was before, but I almost want to combine some of the validators together. Like
```
@IntegerValidator(min=0, nullAllowed=true)
public static final String SOME_CONFIG="some.config";
```
instead of
```
@PositiveIntegerValidator
public static final String SOME_CONFIG="some.config";
```
I'm also not sure about having validator at the end of each annotation. But I don't think we can have a validator named the same as a common class. I want to think this through a bit more.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-146558153
hmmm. You are right, and there is no inheritance in annotations. So we cannot get that to work. Let me think about it a bit more.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-147414223
@revans2 any other suggestions?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-148180252
Filed https://issues.apache.org/jira/browse/STORM-1111 as a follow on to have the correct validation on each of the configs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42044717
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidationAnnotations.java ---
@@ -0,0 +1,216 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.RetentionPolicy;
+
+/**
+ * Note: every annotation interface must have method validatorClass()
+ * For every annotation there must validator class to do the validation
+ * To add another annotation for config validation, add another annotation @interface class. Implement the corresponding
+ * validator logic in a class in ConfigValidation. Make sure validateField method in ConfigValidation knows how to use the validator
+ * and which method definition/parameters to pass in based on what fields are in the annotation.
+ */
+public class ConfigValidationAnnotations {
+ /**
+ * Field names for annotations
+ */
+
+ static final String VALIDATOR_CLASS = "validatorClass";
+ static final String TYPE = "type";
+ static final String ENTRY_VALIDATOR_CLASSES = "entryValidatorClasses";
+ static final String KEY_VALIDATOR_CLASSES = "keyValidatorClasses";
+ static final String VALUE_VALIDATOR_CLASSES = "valueValidatorClasses";
+ static final String KEY_TYPE = "keyType";
+ static final String VALUE_TYPE = "valueType";
+ static final String INCLUDE_ZERO = "includeZero";
+
+ /**
+ * Validators with fields: validatorClass and type
+ */
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target(ElementType.FIELD)
+ public @interface isType {
+ Class validatorClass() default ConfigValidation.SimpleTypeValidator.class;
+
+ Class type();
+ }
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target(ElementType.FIELD)
+ public @interface isStrings {
--- End diff --
Perhaps isStringList so it matches isStringOrStringList.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-146953738
Crap OK. The JSR has thwarted my plan to take over the universe. What we have now looks like it is as good as it gets.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-146949145
Why can't the default be null?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r41399786
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,359 @@
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Created by jerrypeng on 10/1/15.
--- End diff --
will add
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42144647
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,570 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public Validator(Map<String, Object> params) {}
+ public Validator() {}
+ public abstract void validateField(String name, Object o) throws InstantiationException, IllegalAccessException;
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends Validator {
+
+ private Class type;
+
+ public SimpleTypeValidator(Map<String, Object> params) {
+ this.type = (Class) params.get(ConfigValidationAnnotations.ValidatorParams.TYPE);
+ }
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateField(name, this.type, o);
+ }
+
+ public static void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class ImpersonationAclValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no duplicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements. Duplicated element: " + o);
+ }
+ objectSet.add(o);
+ }
+ }
+ }
+
+ /**
+ * Validates a String or a list of Strings
+ */
+ public static class StringOrStringListValidator extends Validator {
+
+ private ConfigValidationUtils.FieldValidator fv = ConfigValidationUtils.listFv(String.class, false);
+
+ @Override
+ public void validateField(String name, Object o) {
+
+ if (o == null) {
+ return;
+ }
+ if (o instanceof String) {
+ return;
+ }
+ //check if iterable
+ try {
+ SimpleTypeValidator.validateField(name, Iterable.class, o);
+ } catch (Exception ex) {
--- End diff --
Why are we ignoring the exception?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42014003
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidationAnnotations.java ---
@@ -0,0 +1,205 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
--- End diff --
I see some in the latest commit. It would be nice to remove them.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-146248606
Well for some for annotations I call a common validator and just pass in the type that needs to be validated, such as @String @Boolean validators.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42014597
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidationAnnotations.java ---
@@ -0,0 +1,205 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
--- End diff --
will remove again. Dane it intellij
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r41886669
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidationAnnotations.java ---
@@ -0,0 +1,205 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
--- End diff --
There are several places where it seems an IDE has added these HTML tags to the license header.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r41898760
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidationAnnotations.java ---
@@ -0,0 +1,205 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
--- End diff --
will remove
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42042633
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements");
--- End diff --
Let's go ahead and include the value that was duplicated too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42027747
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements");
+ }
+ objectSet.add(o);
+ }
+ }
+ }
+
+ /**
+ * Validates a String or a list of Strings
+ */
+ public static class StringOrStringListValidator extends Validator {
+
+ private ConfigValidationUtils.FieldValidator fv = ConfigValidationUtils.listFv(String.class, false);
+
+ @Override
+ public void validateField(String name, Object o) {
+
+ if (o == null) {
+ return;
+ }
+ if (o instanceof String) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ try {
+ isIterable.validateField(name, Iterable.class, o);
+ } catch (Exception ex) {
+ }
+ this.fv.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates Kryo Registration
+ */
+ public static class KryoRegValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Iterable) {
+ for (Object e : (Iterable) o) {
+ if (e instanceof Map) {
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) e).entrySet()) {
+ if (!(entry.getKey() instanceof String) ||
+ !(entry.getValue() instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ } else if (!(e instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ return;
+ }
+ throw new IllegalArgumentException(
+ "Field " + name + " must be an Iterable containing only Strings or Maps of Strings");
+ }
+ }
+
+ /**
+ * Validates if a number is a power of 2
+ */
+ public static class PowerOf2Validator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ // Test whether the integer is a power of 2.
+ if (i > 0 && (i & (i - 1)) == 0) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be a power of 2.");
+ }
+ }
+
+ /**
+ * Validates each entry in a list
+ */
+ public static class ListEntryTypeValidator extends TypeValidator {
+
+ @Override
+ public void validateField(String name, Class type, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.listFv(type, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates entry in the a list with a list of custom validator
+ */
+ public static class ListEntryCustomValidator {
+
+ public void validateField(String name, Class[] validators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, o);
+ for (Object entry : (Iterable) o) {
+ for (Class validator : validators) {
+ Object v = validator.newInstance();
+ if (v instanceof Validator) {
+ ((Validator) v).validateField(name + " list entry", entry);
+ } else {
+ LOG.warn("validator: {} cannot be used in ListEntryCustomValidator. Individual entry validators must a instance of Validator class", validator.getName());
+ }
+ }
+ }
+
+ }
+ }
+
+ /**
+ * validates each key and value in a map of a certain type
+ */
+ public static class MapEntryTypeValidator {
+
+ public void validateField(String name, Class keyType, Class valueType, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(keyType, valueType, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ public static class MapEntryCustomValidator {
+
+ public void validateField(String name, Class[] keyValidators, Class[] valueValidators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if Map
+ SimpleTypeValidator isMap = new SimpleTypeValidator();
+ isMap.validateField(name, Map.class, o);
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) o).entrySet()) {
+ for (Class kv : keyValidators) {
+ Object keyValidator = kv.newInstance();
+ if (keyValidator instanceof Validator) {
+ ((Validator) keyValidator).validateField(name + " Map key", entry.getKey());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate keys. Individual entry validators must a instance of Validator class", kv.getName());
+ }
+ }
+ for (Class vv : valueValidators) {
+ Object valueValidator = vv.newInstance();
+ if (valueValidator instanceof Validator) {
+ ((Validator) valueValidator).validateField(name + " Map value", entry.getValue());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate values. Individual entry validators must a instance of Validator class", vv.getName());
+ }
+ }
+ }
+ }
+ }
+
+ /**
+ * Validates a positive number
+ */
+ public static class PositiveNumberValidator extends Validator{
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateField(name, false, o);
+ }
+
+ public void validateField(String name, boolean includeZero, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Number) {
+ if(includeZero == true) {
--- End diff --
Don't need `== true` here
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/785
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-146248821
Writing another validator like IntegerValidator(min=0, nullAllowed=true) is a trivial task, but I think we need to first focus on setting a standard of using annotations in general. There could be a lot of refactoring done for the validator's but I am not sure if that is in the scope of this Jira
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r41399571
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,359 @@
+package backtype.storm.validation;
--- End diff --
I created another package because I added a couple other files related to config validation
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-146949595
the java annotation spec was written that way
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42028131
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements");
+ }
+ objectSet.add(o);
+ }
+ }
+ }
+
+ /**
+ * Validates a String or a list of Strings
+ */
+ public static class StringOrStringListValidator extends Validator {
+
+ private ConfigValidationUtils.FieldValidator fv = ConfigValidationUtils.listFv(String.class, false);
+
+ @Override
+ public void validateField(String name, Object o) {
+
+ if (o == null) {
+ return;
+ }
+ if (o instanceof String) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ try {
+ isIterable.validateField(name, Iterable.class, o);
+ } catch (Exception ex) {
+ }
+ this.fv.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates Kryo Registration
+ */
+ public static class KryoRegValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Iterable) {
+ for (Object e : (Iterable) o) {
+ if (e instanceof Map) {
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) e).entrySet()) {
+ if (!(entry.getKey() instanceof String) ||
+ !(entry.getValue() instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ } else if (!(e instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ return;
+ }
+ throw new IllegalArgumentException(
+ "Field " + name + " must be an Iterable containing only Strings or Maps of Strings");
+ }
+ }
+
+ /**
+ * Validates if a number is a power of 2
+ */
+ public static class PowerOf2Validator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ // Test whether the integer is a power of 2.
+ if (i > 0 && (i & (i - 1)) == 0) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be a power of 2.");
+ }
+ }
+
+ /**
+ * Validates each entry in a list
+ */
+ public static class ListEntryTypeValidator extends TypeValidator {
+
+ @Override
+ public void validateField(String name, Class type, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.listFv(type, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates entry in the a list with a list of custom validator
+ */
+ public static class ListEntryCustomValidator {
+
+ public void validateField(String name, Class[] validators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, o);
+ for (Object entry : (Iterable) o) {
+ for (Class validator : validators) {
+ Object v = validator.newInstance();
+ if (v instanceof Validator) {
+ ((Validator) v).validateField(name + " list entry", entry);
+ } else {
+ LOG.warn("validator: {} cannot be used in ListEntryCustomValidator. Individual entry validators must a instance of Validator class", validator.getName());
+ }
+ }
+ }
+
+ }
+ }
+
+ /**
+ * validates each key and value in a map of a certain type
+ */
+ public static class MapEntryTypeValidator {
+
+ public void validateField(String name, Class keyType, Class valueType, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(keyType, valueType, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ public static class MapEntryCustomValidator {
+
+ public void validateField(String name, Class[] keyValidators, Class[] valueValidators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if Map
+ SimpleTypeValidator isMap = new SimpleTypeValidator();
+ isMap.validateField(name, Map.class, o);
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) o).entrySet()) {
+ for (Class kv : keyValidators) {
+ Object keyValidator = kv.newInstance();
+ if (keyValidator instanceof Validator) {
+ ((Validator) keyValidator).validateField(name + " Map key", entry.getKey());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate keys. Individual entry validators must a instance of Validator class", kv.getName());
+ }
+ }
+ for (Class vv : valueValidators) {
+ Object valueValidator = vv.newInstance();
+ if (valueValidator instanceof Validator) {
+ ((Validator) valueValidator).validateField(name + " Map value", entry.getValue());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate values. Individual entry validators must a instance of Validator class", vv.getName());
+ }
+ }
+ }
+ }
+ }
+
+ /**
+ * Validates a positive number
+ */
+ public static class PositiveNumberValidator extends Validator{
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateField(name, false, o);
+ }
+
+ public void validateField(String name, boolean includeZero, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Number) {
+ if(includeZero == true) {
--- End diff --
will fix
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42148413
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
--- End diff --
Looks good to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-148473108
@revans2 @d2r Thank you for spending the time to review my PR! Much appreciated!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r41898751
--- Diff: storm-core/test/jvm/backtype/storm/TestConfigValidate.java ---
@@ -0,0 +1,137 @@
+package backtype.storm;
+
+import backtype.storm.utils.Utils;
+import org.junit.Test;
+import org.junit.Assert;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import backtype.storm.validation.ConfigValidationAnnotations;
+import backtype.storm.validation.ConfigValidation;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.Map;
+
+/**
+ * Created by jerrypeng on 10/1/15.
+ */
--- End diff --
will remove
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42042786
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements");
--- End diff --
will add
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-148425413
@d2r @revans2 I just pushed some updates based on what we discussed. Can you guys look at it again? thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42045456
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidationAnnotations.java ---
@@ -0,0 +1,216 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.RetentionPolicy;
+
+/**
+ * Note: every annotation interface must have method validatorClass()
+ * For every annotation there must validator class to do the validation
+ * To add another annotation for config validation, add another annotation @interface class. Implement the corresponding
+ * validator logic in a class in ConfigValidation. Make sure validateField method in ConfigValidation knows how to use the validator
+ * and which method definition/parameters to pass in based on what fields are in the annotation.
+ */
+public class ConfigValidationAnnotations {
+ /**
+ * Field names for annotations
+ */
+
+ static final String VALIDATOR_CLASS = "validatorClass";
+ static final String TYPE = "type";
+ static final String ENTRY_VALIDATOR_CLASSES = "entryValidatorClasses";
+ static final String KEY_VALIDATOR_CLASSES = "keyValidatorClasses";
+ static final String VALUE_VALIDATOR_CLASSES = "valueValidatorClasses";
+ static final String KEY_TYPE = "keyType";
+ static final String VALUE_TYPE = "valueType";
+ static final String INCLUDE_ZERO = "includeZero";
+
+ /**
+ * Validators with fields: validatorClass and type
+ */
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target(ElementType.FIELD)
+ public @interface isType {
+ Class validatorClass() default ConfigValidation.SimpleTypeValidator.class;
+
+ Class type();
+ }
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target(ElementType.FIELD)
+ public @interface isStrings {
--- End diff --
sounds good, will fix
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-146926754
@jerrypeng Perhaps we can do a hybrid approach, where we have validation for complex types made up of something that is a bit ugly, but expressive. And a set of simple annotations for common types. for example
```
@Validate(type=Map.class, nullAllowed=false,
key=@Validate(type=String.class),
value=@Validate(type=Number.class, min=0, max=500))
public static final String MY_MAP_CONF="my.map.conf";
@Validate(type=List.class, value=@Validate(type=String.class))
public static final String MY_LIST_OF_STRINGS="my.list.of.strings";
@IntValidator(min=0, nullAllowed=false)
public static final String MY_INT_CONF="my.int.conf";
//And for the really strange types
@Validate(type=List.class, value=@Validate(type=Map.class, validator=RegistryValidator.class))
public static final String TOPOLOGY_METRICS_CONSUMER_REGISTER = "topology.metrics.consumer.register";
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42148564
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements");
+ }
+ objectSet.add(o);
+ }
+ }
+ }
+
+ /**
+ * Validates a String or a list of Strings
+ */
+ public static class StringOrStringListValidator extends Validator {
+
+ private ConfigValidationUtils.FieldValidator fv = ConfigValidationUtils.listFv(String.class, false);
+
+ @Override
+ public void validateField(String name, Object o) {
+
+ if (o == null) {
+ return;
+ }
+ if (o instanceof String) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ try {
+ isIterable.validateField(name, Iterable.class, o);
+ } catch (Exception ex) {
+ }
+ this.fv.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates Kryo Registration
+ */
+ public static class KryoRegValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Iterable) {
+ for (Object e : (Iterable) o) {
+ if (e instanceof Map) {
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) e).entrySet()) {
+ if (!(entry.getKey() instanceof String) ||
+ !(entry.getValue() instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ } else if (!(e instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ return;
+ }
+ throw new IllegalArgumentException(
+ "Field " + name + " must be an Iterable containing only Strings or Maps of Strings");
+ }
+ }
+
+ /**
+ * Validates if a number is a power of 2
+ */
+ public static class PowerOf2Validator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ // Test whether the integer is a power of 2.
+ if (i > 0 && (i & (i - 1)) == 0) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be a power of 2.");
+ }
+ }
+
+ /**
+ * Validates each entry in a list
+ */
+ public static class ListEntryTypeValidator extends TypeValidator {
+
+ @Override
+ public void validateField(String name, Class type, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.listFv(type, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates entry in the a list with a list of custom validator
+ */
+ public static class ListEntryCustomValidator {
+
+ public void validateField(String name, Class[] validators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, o);
+ for (Object entry : (Iterable) o) {
+ for (Class validator : validators) {
+ Object v = validator.newInstance();
+ if (v instanceof Validator) {
+ ((Validator) v).validateField(name + " list entry", entry);
+ } else {
+ LOG.warn("validator: {} cannot be used in ListEntryCustomValidator. Individual entry validators must a instance of Validator class", validator.getName());
+ }
+ }
+ }
+
+ }
+ }
+
+ /**
+ * validates each key and value in a map of a certain type
+ */
+ public static class MapEntryTypeValidator {
+
+ public void validateField(String name, Class keyType, Class valueType, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(keyType, valueType, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ public static class MapEntryCustomValidator {
+
+ public void validateField(String name, Class[] keyValidators, Class[] valueValidators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if Map
+ SimpleTypeValidator isMap = new SimpleTypeValidator();
+ isMap.validateField(name, Map.class, o);
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) o).entrySet()) {
+ for (Class kv : keyValidators) {
+ Object keyValidator = kv.newInstance();
+ if (keyValidator instanceof Validator) {
+ ((Validator) keyValidator).validateField(name + " Map key", entry.getKey());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate keys. Individual entry validators must a instance of Validator class", kv.getName());
+ }
+ }
+ for (Class vv : valueValidators) {
+ Object valueValidator = vv.newInstance();
+ if (valueValidator instanceof Validator) {
+ ((Validator) valueValidator).validateField(name + " Map value", entry.getValue());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate values. Individual entry validators must a instance of Validator class", vv.getName());
+ }
+ }
+ }
+ }
+ }
+
+ /**
+ * Validates a positive number
+ */
+ public static class PositiveNumberValidator extends Validator{
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateField(name, false, o);
+ }
+
+ public void validateField(String name, boolean includeZero, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Number) {
+ if(includeZero == true) {
+ if (((Number) o).doubleValue() >= 0.0) {
+ return;
+ }
+ } else {
+ if (((Number) o).doubleValue() > 0.0) {
+ return;
+ }
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be a Positive Number");
+ }
+ }
+
+ /**
+ * Methods for validating confs
+ */
+
+ /**
+ * Validates a field given field name as string
+ *
+ * @param fieldName provided as a string
+ * @param conf map of confs
+ */
+ public static void validateField(String fieldName, Map conf) throws NoSuchFieldException, IllegalAccessException, InstantiationException, NoSuchMethodException, InvocationTargetException {
+ validateField(fieldName, conf, CONFIG_CLASS);
+ }
--- End diff --
OK good answer. Let's leave it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-148441476
It looks really good. I am +1 for merging this in. I have one question about checking that they type is iterable in one place, and then ignoring the error, to have the other code blow up if it is not iterable. It should do the right thing, just the code is confusing and/or unnecessary.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42046068
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,518 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public abstract void validateField(String name, Object o);
+ }
+
+ public abstract static class TypeValidator {
+ public abstract void validateField(String name, Class type, Object o);
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends TypeValidator {
+
+ public void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator validator = new SimpleTypeValidator();
+ validator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class MapOfStringToMapValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no dupicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements");
+ }
+ objectSet.add(o);
+ }
+ }
+ }
+
+ /**
+ * Validates a String or a list of Strings
+ */
+ public static class StringOrStringListValidator extends Validator {
+
+ private ConfigValidationUtils.FieldValidator fv = ConfigValidationUtils.listFv(String.class, false);
+
+ @Override
+ public void validateField(String name, Object o) {
+
+ if (o == null) {
+ return;
+ }
+ if (o instanceof String) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ try {
+ isIterable.validateField(name, Iterable.class, o);
+ } catch (Exception ex) {
+ }
+ this.fv.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates Kryo Registration
+ */
+ public static class KryoRegValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Iterable) {
+ for (Object e : (Iterable) o) {
+ if (e instanceof Map) {
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) e).entrySet()) {
+ if (!(entry.getKey() instanceof String) ||
+ !(entry.getValue() instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ } else if (!(e instanceof String)) {
+ throw new IllegalArgumentException(
+ "Each element of the list " + name + " must be a String or a Map of Strings");
+ }
+ }
+ return;
+ }
+ throw new IllegalArgumentException(
+ "Field " + name + " must be an Iterable containing only Strings or Maps of Strings");
+ }
+ }
+
+ /**
+ * Validates if a number is a power of 2
+ */
+ public static class PowerOf2Validator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ // Test whether the integer is a power of 2.
+ if (i > 0 && (i & (i - 1)) == 0) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be a power of 2.");
+ }
+ }
+
+ /**
+ * Validates each entry in a list
+ */
+ public static class ListEntryTypeValidator extends TypeValidator {
+
+ @Override
+ public void validateField(String name, Class type, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.listFv(type, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * Validates entry in the a list with a list of custom validator
+ */
+ public static class ListEntryCustomValidator {
+
+ public void validateField(String name, Class[] validators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator isIterable = new SimpleTypeValidator();
+ isIterable.validateField(name, Iterable.class, o);
+ for (Object entry : (Iterable) o) {
+ for (Class validator : validators) {
+ Object v = validator.newInstance();
+ if (v instanceof Validator) {
+ ((Validator) v).validateField(name + " list entry", entry);
+ } else {
+ LOG.warn("validator: {} cannot be used in ListEntryCustomValidator. Individual entry validators must a instance of Validator class", validator.getName());
+ }
+ }
+ }
+
+ }
+ }
+
+ /**
+ * validates each key and value in a map of a certain type
+ */
+ public static class MapEntryTypeValidator {
+
+ public void validateField(String name, Class keyType, Class valueType, Object o) {
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(keyType, valueType, false);
+ validator.validateField(name, o);
+ }
+ }
+
+ public static class MapEntryCustomValidator {
+
+ public void validateField(String name, Class[] keyValidators, Class[] valueValidators, Object o) throws IllegalAccessException, InstantiationException {
+ if (o == null) {
+ return;
+ }
+ //check if Map
+ SimpleTypeValidator isMap = new SimpleTypeValidator();
+ isMap.validateField(name, Map.class, o);
+ for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) o).entrySet()) {
+ for (Class kv : keyValidators) {
+ Object keyValidator = kv.newInstance();
+ if (keyValidator instanceof Validator) {
+ ((Validator) keyValidator).validateField(name + " Map key", entry.getKey());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate keys. Individual entry validators must a instance of Validator class", kv.getName());
+ }
+ }
+ for (Class vv : valueValidators) {
+ Object valueValidator = vv.newInstance();
+ if (valueValidator instanceof Validator) {
+ ((Validator) valueValidator).validateField(name + " Map value", entry.getValue());
+ } else {
+ LOG.warn("validator: {} cannot be used in MapEntryCustomValidator to validate values. Individual entry validators must a instance of Validator class", vv.getName());
+ }
+ }
+ }
+ }
+ }
+
+ /**
+ * Validates a positive number
+ */
+ public static class PositiveNumberValidator extends Validator{
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateField(name, false, o);
+ }
+
+ public void validateField(String name, boolean includeZero, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (o instanceof Number) {
+ if(includeZero == true) {
+ if (((Number) o).doubleValue() >= 0.0) {
+ return;
+ }
+ } else {
+ if (((Number) o).doubleValue() > 0.0) {
+ return;
+ }
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be a Positive Number");
+ }
+ }
+
+ /**
+ * Methods for validating confs
+ */
+
+ /**
+ * Validates a field given field name as string
+ *
+ * @param fieldName provided as a string
+ * @param conf map of confs
+ */
+ public static void validateField(String fieldName, Map conf) throws NoSuchFieldException, IllegalAccessException, InstantiationException, NoSuchMethodException, InvocationTargetException {
+ validateField(fieldName, conf, CONFIG_CLASS);
+ }
--- End diff --
Is this method needed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-148442889
I am +1 pending not related CI failures.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-147752280
@knusbaum @d2r @zhuoliu @rfarivar Can review this PR? Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42032492
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidationAnnotations.java ---
@@ -0,0 +1,216 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.RetentionPolicy;
+
+/**
+ * Note: every annotation interface must have method validatorClass()
+ * For every annotation there must validator class to do the validation
+ * To add another annotation for config validation, add another annotation @interface class. Implement the corresponding
+ * validator logic in a class in ConfigValidation. Make sure validateField method in ConfigValidation knows how to use the validator
+ * and which method definition/parameters to pass in based on what fields are in the annotation.
+ */
+public class ConfigValidationAnnotations {
+ /**
+ * Field names for annotations
+ */
+
+ static final String VALIDATOR_CLASS = "validatorClass";
+ static final String TYPE = "type";
+ static final String ENTRY_VALIDATOR_CLASSES = "entryValidatorClasses";
+ static final String KEY_VALIDATOR_CLASSES = "keyValidatorClasses";
+ static final String VALUE_VALIDATOR_CLASSES = "valueValidatorClasses";
+ static final String KEY_TYPE = "keyType";
+ static final String VALUE_TYPE = "valueType";
+ static final String INCLUDE_ZERO = "includeZero";
+
+ /**
+ * Validators with fields: validatorClass and type
+ */
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target(ElementType.FIELD)
+ public @interface isType {
+ Class validatorClass() default ConfigValidation.SimpleTypeValidator.class;
+
+ Class type();
+ }
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @Target(ElementType.FIELD)
+ public @interface isStrings {
--- End diff --
ok how about isListOfStrings then? so to keep the "is" prefix consistent
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by jerrypeng <gi...@git.apache.org>.
Github user jerrypeng commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r42145138
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,570 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package backtype.storm.validation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+
+/**
+ * Provides functionality for validating configuration fields.
+ */
+public class ConfigValidation {
+
+ private static final Class CONFIG_CLASS = backtype.storm.Config.class;
+
+ private static final Logger LOG = LoggerFactory.getLogger(ConfigValidation.class);
+
+ public static abstract class Validator {
+ public Validator(Map<String, Object> params) {}
+ public Validator() {}
+ public abstract void validateField(String name, Object o) throws InstantiationException, IllegalAccessException;
+ }
+
+ /**
+ * Validator definitions
+ */
+
+ /**
+ * Validates if an object is not null
+ */
+
+ public static class NotNullValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Field " + name + "cannot be null! Actual value: " + o);
+ }
+ }
+ }
+
+ /**
+ * Validates basic types
+ */
+ public static class SimpleTypeValidator extends Validator {
+
+ private Class type;
+
+ public SimpleTypeValidator(Map<String, Object> params) {
+ this.type = (Class) params.get(ConfigValidationAnnotations.ValidatorParams.TYPE);
+ }
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateField(name, this.type, o);
+ }
+
+ public static void validateField(String name, Class type, Object o) {
+ if (o == null) {
+ return;
+ }
+ if (type.isInstance(o)) {
+ return;
+ }
+ throw new IllegalArgumentException("Field " + name + " must be of type " + type + ". Object: " + o + " actual type: " + o.getClass());
+ }
+ }
+
+ public static class StringValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator.validateField(name, String.class, o);
+ }
+ }
+
+ public static class BooleanValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator.validateField(name, Boolean.class, o);
+ }
+ }
+
+ public static class NumberValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator.validateField(name, Number.class, o);
+ }
+ }
+
+ public static class DoubleValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ SimpleTypeValidator.validateField(name, Double.class, o);
+ }
+ }
+
+ /**
+ * Validates a Integer.
+ */
+ public static class IntegerValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ validateInteger(name, o);
+ }
+
+ public void validateInteger(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ final long i;
+ if (o instanceof Number &&
+ (i = ((Number) o).longValue()) == ((Number) o).doubleValue()) {
+ if (i <= Integer.MAX_VALUE && i >= Integer.MIN_VALUE) {
+ return;
+ }
+ }
+ throw new IllegalArgumentException("Field " + name + " must be an Integer within type range.");
+ }
+ }
+
+ /**
+ * Validates a map of Strings to a map of Strings to a list.
+ * {str -> {str -> [str,str]}
+ */
+ public static class ImpersonationAclValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object o) {
+ if (o == null) {
+ return;
+ }
+ ConfigValidationUtils.NestableFieldValidator validator = ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.mapFv(ConfigValidationUtils.fv(String.class, false),
+ ConfigValidationUtils.listFv(String.class, false), false), true);
+ validator.validateField(name, o);
+ }
+ }
+
+ /**
+ * validates a list of has no duplicates
+ */
+ public static class NoDuplicateInListValidator extends Validator {
+
+ @Override
+ public void validateField(String name, Object field) {
+ if (field == null) {
+ return;
+ }
+ //check if iterable
+ SimpleTypeValidator.validateField(name, Iterable.class, field);
+ HashSet<Object> objectSet = new HashSet<Object>();
+ for (Object o : (Iterable) field) {
+ if (objectSet.contains(o)) {
+ throw new IllegalArgumentException(name + " should contain no duplicate elements. Duplicated element: " + o);
+ }
+ objectSet.add(o);
+ }
+ }
+ }
+
+ /**
+ * Validates a String or a list of Strings
+ */
+ public static class StringOrStringListValidator extends Validator {
+
+ private ConfigValidationUtils.FieldValidator fv = ConfigValidationUtils.listFv(String.class, false);
+
+ @Override
+ public void validateField(String name, Object o) {
+
+ if (o == null) {
+ return;
+ }
+ if (o instanceof String) {
+ return;
+ }
+ //check if iterable
+ try {
+ SimpleTypeValidator.validateField(name, Iterable.class, o);
+ } catch (Exception ex) {
--- End diff --
that is not suppose to be in there. Not sure how the try catch statement got in there to begin with. Will remove
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-1084] - Improve Storm config validation...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/785#discussion_r41397418
--- Diff: storm-core/src/jvm/backtype/storm/validation/ConfigValidation.java ---
@@ -0,0 +1,359 @@
+package backtype.storm.validation;
--- End diff --
Needs that Apache header. Also not completely sure why we changed the package. Not important though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---