From 8e0ba42df43fa6f9a9d42746c7afb0eee0a31ae3 Mon Sep 17 00:00:00 2001 From: Chris Nauroth Date: Fri, 10 Jan 2014 19:57:51 +0000 Subject: [PATCH] HADOOP-10178. Configuration deprecation always emit "deprecated" warnings when a new key is used. Contributed by Shanyu Zhao. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1557236 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 3 + .../org/apache/hadoop/conf/Configuration.java | 136 +++++++++++------- .../conf/TestConfigurationDeprecation.java | 27 ++++ 3 files changed, 112 insertions(+), 54 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index a4446e53ac..e8c7e5cc45 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -596,6 +596,9 @@ Release 2.3.0 - UNRELEASED HADOOP-10193. hadoop-auth's PseudoAuthenticationHandler can consume getInputStream. (gchanan via tucu) + HADOOP-10178. Configuration deprecation always emit "deprecated" warnings + when a new key is used. (Shanyu Zhao via cnauroth) + Release 2.2.0 - 2013-10-13 INCOMPATIBLE CHANGES 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 3ba98aa3be..456a8d2cf1 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 @@ -552,36 +552,6 @@ public static boolean isDeprecated(String key) { return deprecationContext.get().getDeprecatedKeyMap().containsKey(key); } - /** - * Returns the alternate name for a key if the property name is deprecated - * or if deprecates a property name. - * - * @param name property name. - * @return alternate name. - */ - private String[] getAlternateNames(String name) { - String altNames[] = null; - DeprecationContext cur = deprecationContext.get(); - DeprecatedKeyInfo keyInfo = cur.getDeprecatedKeyMap().get(name); - if (keyInfo == null) { - altNames = (cur.getReverseDeprecatedKeyMap().get(name) != null ) ? - new String [] {cur.getReverseDeprecatedKeyMap().get(name)} : null; - if(altNames != null && altNames.length > 0) { - //To help look for other new configs for this deprecated config - keyInfo = cur.getDeprecatedKeyMap().get(altNames[0]); - } - } - if(keyInfo != null && keyInfo.newKeys.length > 0) { - List list = new ArrayList(); - if(altNames != null) { - list.addAll(Arrays.asList(altNames)); - } - list.addAll(Arrays.asList(keyInfo.newKeys)); - altNames = list.toArray(new String[list.size()]); - } - return altNames; - } - /** * Checks for the presence of the property name in the * deprecation map. Returns the first of the list of new keys if present @@ -933,6 +903,37 @@ public String getRaw(String name) { return result; } + /** + * Returns alternative names (non-deprecated keys or previously-set deprecated keys) + * for a given non-deprecated key. + * If the given key is deprecated, return null. + * + * @param name property name. + * @return alternative names. + */ + private String[] getAlternativeNames(String name) { + String altNames[] = null; + DeprecatedKeyInfo keyInfo = null; + DeprecationContext cur = deprecationContext.get(); + String depKey = cur.getReverseDeprecatedKeyMap().get(name); + if(depKey != null) { + keyInfo = cur.getDeprecatedKeyMap().get(depKey); + if(keyInfo.newKeys.length > 0) { + if(getProps().containsKey(depKey)) { + //if deprecated key is previously set explicitly + List list = new ArrayList(); + list.addAll(Arrays.asList(keyInfo.newKeys)); + list.add(depKey); + altNames = list.toArray(new String[list.size()]); + } + else { + altNames = keyInfo.newKeys; + } + } + } + return altNames; + } + /** * Set the value of the name property. If * name is deprecated or there is a deprecated name associated to it, @@ -947,9 +948,9 @@ public void set(String name, String value) { /** * Set the value of the name property. If - * name is deprecated or there is a deprecated name associated to it, - * it sets the value to both names. - * + * name is deprecated, it also sets the value to + * the keys that replace the deprecated key. + * * @param name property name. * @param value property value. * @param source the place that this configuration value came from @@ -969,23 +970,30 @@ public void set(String name, String value, String source) { } getOverlay().setProperty(name, value); getProps().setProperty(name, value); - if(source == null) { - updatingResource.put(name, new String[] {"programatically"}); - } else { - updatingResource.put(name, new String[] {source}); - } - String[] altNames = getAlternateNames(name); - if (altNames != null && altNames.length > 0) { - String altSource = "because " + name + " is deprecated"; - for(String altName : altNames) { - if(!altName.equals(name)) { - getOverlay().setProperty(altName, value); - getProps().setProperty(altName, value); - updatingResource.put(altName, new String[] {altSource}); + String newSource = (source == null ? "programatically" : source); + + if (!isDeprecated(name)) { + updatingResource.put(name, new String[] {newSource}); + String[] altNames = getAlternativeNames(name); + if(altNames != null) { + for(String n: altNames) { + if(!n.equals(name)) { + getOverlay().setProperty(n, value); + getProps().setProperty(n, value); + updatingResource.put(n, new String[] {newSource}); + } } } } - warnOnceIfDeprecated(deprecations, name); + else { + String[] names = handleDeprecation(deprecationContext.get(), name); + String altSource = "because " + name + " is deprecated"; + for(String n : names) { + getOverlay().setProperty(n, value); + getProps().setProperty(n, value); + updatingResource.put(n, new String[] {altSource}); + } + } } private void warnOnceIfDeprecated(DeprecationContext deprecations, String name) { @@ -999,15 +1007,21 @@ private void warnOnceIfDeprecated(DeprecationContext deprecations, String name) * Unset a previously set property. */ public synchronized void unset(String name) { - String[] altNames = getAlternateNames(name); - getOverlay().remove(name); - getProps().remove(name); - if (altNames !=null && altNames.length > 0) { - for(String altName : altNames) { - getOverlay().remove(altName); - getProps().remove(altName); + String[] names = null; + if (!isDeprecated(name)) { + names = getAlternativeNames(name); + if(names == null) { + names = new String[]{name}; } } + else { + names = handleDeprecation(deprecationContext.get(), name); + } + + for(String n: names) { + getOverlay().remove(n); + getProps().remove(n); + } } /** @@ -2600,4 +2614,18 @@ public static void dumpDeprecatedKeys() { System.out.println(entry.getKey() + "\t" + newKeys.toString()); } } + + /** + * Returns whether or not a deprecated name has been warned. If the name is not + * deprecated then always return false + */ + public static boolean hasWarnedDeprecation(String name) { + DeprecationContext deprecations = deprecationContext.get(); + if(deprecations.getDeprecatedKeyMap().containsKey(name)) { + if(deprecations.getDeprecatedKeyMap().get(name).accessed.get()) { + return true; + } + } + return false; + } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java index bdd22cd0d3..f336821620 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationDeprecation.java @@ -26,6 +26,7 @@ import java.io.File; import java.io.FileWriter; import java.io.IOException; +import java.io.ByteArrayOutputStream; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -399,4 +400,30 @@ public Void call() throws Exception { Uninterruptibles.getUninterruptibly(future); } } + + @Test + public void testNoFalseDeprecationWarning() throws IOException { + Configuration conf = new Configuration(); + Configuration.addDeprecation("AA", "BB"); + conf.set("BB", "bb"); + conf.get("BB"); + conf.writeXml(new ByteArrayOutputStream()); + assertEquals(false, Configuration.hasWarnedDeprecation("AA")); + conf.set("AA", "aa"); + assertEquals(true, Configuration.hasWarnedDeprecation("AA")); + } + + @Test + public void testDeprecationSetUnset() throws IOException { + addDeprecationToConfiguration(); + Configuration conf = new Configuration(); + //"X" is deprecated by "Y" and "Z" + conf.set("Y", "y"); + assertEquals("y", conf.get("Z")); + conf.set("X", "x"); + assertEquals("x", conf.get("Z")); + conf.unset("Y"); + assertEquals(null, conf.get("Z")); + assertEquals(null, conf.get("X")); + } }