[
https://issues.apache.org/jira/browse/STORM-1084?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14959138#comment-14959138
]
ASF GitHub Bot commented on STORM-1084:
---------------------------------------
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?
> 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)
