[
https://issues.apache.org/jira/browse/STORM-1084?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14957650#comment-14957650
]
ASF GitHub Bot commented on STORM-1084:
---------------------------------------
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`.
> Improve Storm config validation process to use java annotations instead of
> *_SCHEMA format
> ------------------------------------------------------------------------------------------
>
> Key: STORM-1084
> URL: https://issues.apache.org/jira/browse/STORM-1084
> Project: Apache Storm
> Issue Type: Improvement
> Components: storm-core
> Reporter: Boyang Jerry Peng
> Assignee: Boyang Jerry Peng
>
> 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
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
