diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 91d4e685f3d..63718d5aa15 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -94,6 +94,9 @@ Release 2.7.0 - UNRELEASED HDFS-7555. Remove the support of unmanaged connectors in HttpServer2. (wheat9) + HADOOP-11399. Java Configuration file and .xml files should be + automatically cross-compared (rchiang via rkanter) + OPTIMIZATIONS HADOOP-11323. WritableComparator#compare keeps reference to byte array. diff --git a/hadoop-common-project/hadoop-common/dev-support/findbugsExcludeFile.xml b/hadoop-common-project/hadoop-common/dev-support/findbugsExcludeFile.xml index e03d3b9372a..4a8cbafda45 100644 --- a/hadoop-common-project/hadoop-common/dev-support/findbugsExcludeFile.xml +++ b/hadoop-common-project/hadoop-common/dev-support/findbugsExcludeFile.xml @@ -404,4 +404,10 @@ + + + + + + diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java index 3840a131be0..387e64a78a7 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java @@ -18,6 +18,8 @@ package org.apache.hadoop.conf; +import com.google.common.annotations.VisibleForTesting; + import java.io.BufferedInputStream; import java.io.DataInput; import java.io.DataOutput; @@ -178,6 +180,11 @@ public class Configuration implements Iterable>, LogFactory.getLog("org.apache.hadoop.conf.Configuration.deprecation"); private boolean quietmode = true; + + private static final String DEFAULT_STRING_CHECK = + "testingforemptydefaultvalue"; + + private boolean allowNullValueProperties = false; private static class Resource { private final Object resource; @@ -890,7 +897,38 @@ public class Configuration implements Iterable>, } return result; } - + + /** + * Set Configuration to allow keys without values during setup. Intended + * for use during testing. + * + * @param val If true, will allow Configuration to store keys without values + */ + @VisibleForTesting + public void setAllowNullValueProperties( boolean val ) { + this.allowNullValueProperties = val; + } + + /** + * Return existence of the name property, but only for + * names which have no valid value, usually non-existent or commented + * out in XML. + * + * @param name the property name + * @return true if the property name exists without value + */ + @VisibleForTesting + public boolean onlyKeyExists(String name) { + String[] names = handleDeprecation(deprecationContext.get(), name); + for(String n : names) { + if ( getProps().getProperty(n,DEFAULT_STRING_CHECK) + .equals(DEFAULT_STRING_CHECK) ) { + return true; + } + } + return false; + } + /** * Get the value of the name property as a trimmed String, * null if no such property exists. @@ -2319,9 +2357,9 @@ public class Configuration implements Iterable>, // code. Map result = new HashMap(); for(Map.Entry item: getProps().entrySet()) { - if (item.getKey() instanceof String && + if (item.getKey() instanceof String && item.getValue() instanceof String) { - result.put((String) item.getKey(), (String) item.getValue()); + result.put((String) item.getKey(), (String) item.getValue()); } } return result.entrySet().iterator(); @@ -2527,8 +2565,11 @@ public class Configuration implements Iterable>, private void loadProperty(Properties properties, String name, String attr, String value, boolean finalParameter, String[] source) { - if (value != null) { + if (value != null || allowNullValueProperties) { if (!finalParameters.contains(attr)) { + if (value==null && allowNullValueProperties) { + value = DEFAULT_STRING_CHECK; + } properties.setProperty(attr, value); updatingResource.put(attr, source); } else if (!value.equals(properties.getProperty(attr))) { diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationFieldsBase.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationFieldsBase.java new file mode 100644 index 00000000000..c3fe3a39f5b --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationFieldsBase.java @@ -0,0 +1,417 @@ +/** + * 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 org.apache.hadoop.conf; + +import java.lang.Class; +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Test; +import static org.junit.Assert.assertTrue; + +import org.apache.hadoop.conf.Configuration; + +/** + * Base class for comparing fields in one or more Configuration classes + * against a corresponding .xml file. Usage is intended as follows: + *

+ *
    + *
  1. Create a subclass to TestConfigurationFieldsBase + *
  2. Define initializeMemberVariables method in the + * subclass. In this class, do the following: + *

    + *
      + *
    1. Required Set the variable xmlFilename to + * the appropriate xml definition file + *
    2. Required Set the variable configurationClasses + * to an array of the classes which define the constants used by the + * code corresponding to the xml files + *
    3. Optional Set errorIfMissingConfigProps if the + * subclass should throw an error in the method + * testCompareXmlAgainstConfigurationClass + *
    4. Optional Set errorIfMissingXmlProps if the + * subclass should throw an error in the method + * testCompareConfigurationClassAgainstXml + *
    5. Optional Instantiate and populate strings into one or + * more of the following variables: + *
      configurationPropsToSkipCompare + *
      configurationPrefixToSkipCompare + *
      xmlPropsToSkipCompare + *
      xmlPrefixToSkipCompare + *
      + * in order to get comparisons clean + *
    + *
+ *

+ * The tests to do class-to-file and file-to-class should automatically + * run. This class (and its subclasses) are mostly not intended to be + * overridden, but to do a very specific form of comparison testing. + */ +@Ignore +public abstract class TestConfigurationFieldsBase { + + /** + * Member variable for storing xml filename. + */ + protected String xmlFilename = null; + + /** + * Member variable for storing all related Configuration classes. + */ + protected Class[] configurationClasses = null; + + /** + * Throw error during comparison if missing configuration properties. + * Intended to be set by subclass. + */ + protected boolean errorIfMissingConfigProps = false; + + /** + * Throw error during comparison if missing xml properties. Intended + * to be set by subclass. + */ + protected boolean errorIfMissingXmlProps = false; + + /** + * Set of properties to skip extracting (and thus comparing later) in + * extractMemberVariablesFromConfigurationFields. + */ + protected Set configurationPropsToSkipCompare = null; + + /** + * Set of property prefixes to skip extracting (and thus comparing later) + * in * extractMemberVariablesFromConfigurationFields. + */ + protected Set configurationPrefixToSkipCompare = null; + + /** + * Set of properties to skip extracting (and thus comparing later) in + * extractPropertiesFromXml. + */ + protected Set xmlPropsToSkipCompare = null; + + /** + * Set of property prefixes to skip extracting (and thus comparing later) + * in extractPropertiesFromXml. + */ + protected Set xmlPrefixToSkipCompare = null; + + /** + * Member variable to store Configuration variables for later comparison. + */ + private Map configurationMemberVariables = null; + + /** + * Member variable to store XML properties for later comparison. + */ + private Map xmlKeyValueMap = null; + + /** + * Member variable to store Configuration variables that are not in the + * corresponding XML file. + */ + private Set configurationFieldsMissingInXmlFile = null; + + /** + * Member variable to store XML variables that are not in the + * corresponding Configuration class(es). + */ + private Set xmlFieldsMissingInConfiguration = null; + + /** + * Abstract method to be used by subclasses for initializing base + * members. + */ + public abstract void initializeMemberVariables(); + + /** + * Utility function to extract "public static final" member + * variables from a Configuration type class. + * + * @param fields The class member variables + * @return HashMap containing entries + */ + private HashMap + extractMemberVariablesFromConfigurationFields(Field[] fields) { + // Sanity Check + if (fields==null) + return null; + + HashMap retVal = new HashMap(); + + // Setup regexp for valid properties + String propRegex = "^[A-Za-z_-]+(\\.[A-Za-z_-]+)+$"; + Pattern p = Pattern.compile(propRegex); + + // Iterate through class member variables + int totalFields = 0; + String value; + for (Field f : fields) { + // Filter out anything that isn't "public static final" + if (!Modifier.isStatic(f.getModifiers()) || + !Modifier.isPublic(f.getModifiers()) || + !Modifier.isFinal(f.getModifiers())) { + continue; + } + // Filter out anything that isn't a string. int/float are generally + // default values + if (!f.getType().getName().equals("java.lang.String")) { + continue; + } + // Convert found member into String + try { + value = (String) f.get(null); + } catch (IllegalAccessException iaException) { + continue; + } + // Special Case: Detect and ignore partial properties (ending in x) + // or file properties (ending in .xml) + if (value.endsWith(".xml") || + value.endsWith(".") || + value.endsWith("-")) + continue; + // Ignore known configuration props + if (configurationPropsToSkipCompare != null) { + if (configurationPropsToSkipCompare.contains(value)) { + continue; + } + } + // Ignore known configuration prefixes + boolean skipPrefix = false; + if (configurationPrefixToSkipCompare != null) { + for (String cfgPrefix : configurationPrefixToSkipCompare) { + if (value.startsWith(cfgPrefix)) { + skipPrefix = true; + break; + } + } + } + if (skipPrefix) { + continue; + } + // Positive Filter: Look only for property values. Expect it to look + // something like: blah.blah2(.blah3.blah4...) + Matcher m = p.matcher(value); + if (!m.find()) { + continue; + } + + // Save member variable/value as hash + retVal.put(value,f.getName()); + } + + return retVal; + } + + /** + * Pull properties and values from filename. + * + * @param filename XML filename + * @return HashMap containing entries from XML file + */ + private HashMap extractPropertiesFromXml + (String filename) { + if (filename==null) { + return null; + } + + // Iterate through XML file for name/value pairs + Configuration conf = new Configuration(false); + conf.setAllowNullValueProperties(true); + conf.addResource(filename); + + HashMap retVal = new HashMap(); + Iterator> kvItr = conf.iterator(); + while (kvItr.hasNext()) { + Map.Entry entry = kvItr.next(); + String key = entry.getKey(); + // Ignore known xml props + if (xmlPropsToSkipCompare != null) { + if (xmlPropsToSkipCompare.contains(key)) { + continue; + } + } + // Ignore known xml prefixes + boolean skipPrefix = false; + if (xmlPrefixToSkipCompare != null) { + for (String xmlPrefix : xmlPrefixToSkipCompare) { + if (key.startsWith(xmlPrefix)) { + skipPrefix = true; + break; + } + } + } + if (skipPrefix) { + continue; + } + if (conf.onlyKeyExists(key)) { + retVal.put(key,null); + } else { + String value = conf.get(key); + if (value!=null) { + retVal.put(key,entry.getValue()); + } + } + kvItr.remove(); + } + return retVal; + } + + /** + * Perform set difference operation on keyMap2 from keyMap1. + * + * @param keyMap1 The initial set + * @param keyMap2 The set to subtract + * @return Returns set operation keyMap1-keyMap2 + */ + private static Set compareConfigurationToXmlFields(Map keyMap1, Map keyMap2) { + Set retVal = new HashSet(keyMap1.keySet()); + retVal.removeAll(keyMap2.keySet()); + return retVal; + } + + /** + * Initialize the four variables corresponding the Configuration + * class and the XML properties file. + */ + @Before + public void setupTestConfigurationFields() throws Exception { + initializeMemberVariables(); + + // Error if subclass hasn't set class members + assertTrue(xmlFilename!=null); + assertTrue(configurationClasses!=null); + + // Create class member/value map + configurationMemberVariables = new HashMap(); + for (Class c : configurationClasses) { + Field[] fields = c.getDeclaredFields(); + Map memberMap = + extractMemberVariablesFromConfigurationFields(fields); + if (memberMap!=null) { + configurationMemberVariables.putAll(memberMap); + } + } + + // Create XML key/value map + xmlKeyValueMap = extractPropertiesFromXml(xmlFilename); + + // Find class members not in the XML file + configurationFieldsMissingInXmlFile = compareConfigurationToXmlFields + (configurationMemberVariables, xmlKeyValueMap); + + // Find XML properties not in the class + xmlFieldsMissingInConfiguration = compareConfigurationToXmlFields + (xmlKeyValueMap, configurationMemberVariables); + } + + /** + * Compares the properties that are in the Configuration class, but not + * in the XML properties file. + */ + @Test + public void testCompareConfigurationClassAgainstXml() { + // Error if subclass hasn't set class members + assertTrue(xmlFilename!=null); + assertTrue(configurationClasses!=null); + + final int missingXmlSize = configurationFieldsMissingInXmlFile.size(); + + for (Class c : configurationClasses) { + System.out.println(c); + } + System.out.println(" (" + configurationMemberVariables.size() + " member variables)"); + System.out.println(); + StringBuffer xmlErrorMsg = new StringBuffer(); + for (Class c : configurationClasses) { + xmlErrorMsg.append(c); + xmlErrorMsg.append(" "); + } + xmlErrorMsg.append("has "); + xmlErrorMsg.append(missingXmlSize); + xmlErrorMsg.append(" variables missing in "); + xmlErrorMsg.append(xmlFilename); + System.out.println(xmlErrorMsg.toString()); + System.out.println(); + if (missingXmlSize==0) { + System.out.println(" (None)"); + } else { + for (String missingField : configurationFieldsMissingInXmlFile) { + System.out.println(" " + missingField); + } + } + System.out.println(); + System.out.println("====="); + System.out.println(); + if (errorIfMissingXmlProps) { + assertTrue(xmlErrorMsg.toString(), missingXmlSize==0); + } + } + + /** + * Compares the properties that are in the XML properties file, but not + * in the Configuration class. + */ + @Test + public void testCompareXmlAgainstConfigurationClass() { + // Error if subclass hasn't set class members + assertTrue(xmlFilename!=null); + assertTrue(configurationClasses!=null); + + final int missingConfigSize = xmlFieldsMissingInConfiguration.size(); + + System.out.println("File " + xmlFilename + " (" + xmlKeyValueMap.size() + " properties)"); + System.out.println(); + StringBuffer configErrorMsg = new StringBuffer(); + configErrorMsg.append(xmlFilename); + configErrorMsg.append(" has "); + configErrorMsg.append(missingConfigSize); + configErrorMsg.append(" properties missing in"); + for (Class c : configurationClasses) { + configErrorMsg.append(" " + c); + } + System.out.println(configErrorMsg.toString()); + System.out.println(); + if (missingConfigSize==0) { + System.out.println(" (None)"); + } else { + for (String missingField : xmlFieldsMissingInConfiguration) { + System.out.println(" " + missingField); + } + } + System.out.println(); + System.out.println("====="); + System.out.println(); + if ( errorIfMissingConfigProps ) { + assertTrue(configErrorMsg.toString(), missingConfigSize==0); + } + } +}