Return-Path: X-Original-To: apmail-cassandra-commits-archive@www.apache.org Delivered-To: apmail-cassandra-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 96728198D7 for ; Wed, 6 Apr 2016 14:43:10 +0000 (UTC) Received: (qmail 67285 invoked by uid 500); 6 Apr 2016 14:43:10 -0000 Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org Received: (qmail 67249 invoked by uid 500); 6 Apr 2016 14:43:10 -0000 Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cassandra.apache.org Delivered-To: mailing list commits@cassandra.apache.org Received: (qmail 67238 invoked by uid 99); 6 Apr 2016 14:43:10 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 06 Apr 2016 14:43:10 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 32726DFDE0; Wed, 6 Apr 2016 14:43:10 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: blerer@apache.org To: commits@cassandra.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: cassandra git commit: Improve field-checking and error reporting in cassandra.yaml Date: Wed, 6 Apr 2016 14:43:10 +0000 (UTC) Repository: cassandra Updated Branches: refs/heads/trunk 83f4e6102 -> ee4bada2f Improve field-checking and error reporting in cassandra.yaml patch by Benjamin Lerer; reviewed by Sylvain Lebresne for CASSANDRA-10649 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/ee4bada2 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/ee4bada2 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/ee4bada2 Branch: refs/heads/trunk Commit: ee4bada2fcbf7ae00f137ac4b7580d4ad50a06a8 Parents: 83f4e61 Author: Benjamin Lerer Authored: Wed Apr 6 16:40:18 2016 +0200 Committer: Benjamin Lerer Committed: Wed Apr 6 16:40:18 2016 +0200 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../config/YamlConfigurationLoader.java | 67 +++++++++++++++++--- 2 files changed, 59 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/ee4bada2/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 3db6b3f..5aa56fe 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.6 + * Improve field-checking and error reporting in cassandra.yaml (CASSANDRA-10649) * Print CAS stats in nodetool proxyhistograms (CASSANDRA-11507) * More user friendly error when providing an invalid token to nodetool (CASSANDRA-9348) * Add static column support to SASI index (CASSANDRA-11183) http://git-wip-us.apache.org/repos/asf/cassandra/blob/ee4bada2/src/java/org/apache/cassandra/config/YamlConfigurationLoader.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/config/YamlConfigurationLoader.java b/src/java/org/apache/cassandra/config/YamlConfigurationLoader.java index 435377c..bd5638a 100644 --- a/src/java/org/apache/cassandra/config/YamlConfigurationLoader.java +++ b/src/java/org/apache/cassandra/config/YamlConfigurationLoader.java @@ -20,10 +20,11 @@ package org.apache.cassandra.config; import java.beans.IntrospectionException; import java.io.ByteArrayInputStream; import java.io.File; -import java.io.InputStream; import java.io.IOException; +import java.io.InputStream; import java.net.URL; import java.util.HashSet; + import java.util.List; import java.util.Map; import java.util.Set; @@ -32,6 +33,9 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.google.common.io.ByteStreams; + +import org.apache.commons.lang3.SystemUtils; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -112,16 +116,17 @@ public class YamlConfigurationLoader implements ConfigurationLoader } Constructor constructor = new CustomConstructor(Config.class); - MissingPropertiesChecker propertiesChecker = new MissingPropertiesChecker(); + PropertiesChecker propertiesChecker = new PropertiesChecker(); constructor.setPropertyUtils(propertiesChecker); Yaml yaml = new Yaml(constructor); - Config result = yaml.loadAs(new ByteArrayInputStream(configBytes), Config.class); + Config result = loadConfig(yaml, configBytes); propertiesChecker.check(); return result; } catch (YAMLException e) { - throw new ConfigurationException("Invalid yaml: " + url, e); + throw new ConfigurationException("Invalid yaml: " + url + SystemUtils.LINE_SEPARATOR + + " Error: " + e.getMessage(), false); } } @@ -161,11 +166,25 @@ public class YamlConfigurationLoader implements ConfigurationLoader } } - private static class MissingPropertiesChecker extends PropertyUtils + private Config loadConfig(Yaml yaml, byte[] configBytes) + { + Config config = yaml.loadAs(new ByteArrayInputStream(configBytes), Config.class); + // If the configuration file is empty yaml will return null. In this case we should use the default + // configuration to avoid hitting a NPE at a later stage. + return config == null ? new Config() : config; + } + + /** + * Utility class to check that there are no extra properties and that properties that are not null by default + * are not set to null. + */ + private static class PropertiesChecker extends PropertyUtils { private final Set missingProperties = new HashSet<>(); - public MissingPropertiesChecker() + private final Set nullProperties = new HashSet<>(); + + public PropertiesChecker() { setSkipMissingProperties(true); } @@ -173,19 +192,49 @@ public class YamlConfigurationLoader implements ConfigurationLoader @Override public Property getProperty(Class type, String name) throws IntrospectionException { - Property result = super.getProperty(type, name); + final Property result = super.getProperty(type, name); + if (result instanceof MissingProperty) { missingProperties.add(result.getName()); } - return result; + + return new Property(result.getName(), result.getType()) + { + @Override + public void set(Object object, Object value) throws Exception + { + if (value == null && get(object) != null) + { + nullProperties.add(getName()); + } + result.set(object, value); + } + + @Override + public Class[] getActualTypeArguments() + { + return result.getActualTypeArguments(); + } + + @Override + public Object get(Object object) + { + return result.get(object); + } + }; } public void check() throws ConfigurationException { + if (!nullProperties.isEmpty()) + { + throw new ConfigurationException("Invalid yaml. Those properties " + nullProperties + " are not valid", false); + } + if (!missingProperties.isEmpty()) { - throw new ConfigurationException("Invalid yaml. Please remove properties " + missingProperties + " from your cassandra.yaml"); + throw new ConfigurationException("Invalid yaml. Please remove properties " + missingProperties + " from your cassandra.yaml", false); } } }