This is an automated email from the ASF dual-hosted git repository.

zuston pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git


The following commit(s) were added to refs/heads/master by this push:
     new 24a51954 [#841] feat(config): Support deprecated and fallback keys for 
ConfigOptions (#843)
24a51954 is described below

commit 24a51954aaa72633fb6640a7195000adbc9034b1
Author: Junfan Zhang <[email protected]>
AuthorDate: Thu Apr 27 13:19:48 2023 +0800

    [#841] feat(config): Support deprecated and fallback keys for ConfigOptions 
(#843)
    
    ### What changes were proposed in this pull request?
    
    Support deprecated and fallback keys for ConfigOptions
    
    ### Why are the changes needed?
    
    #841 . To achieve better compatibility, it's time to support deprecated 
keys for ConfigOptions .
    
    After this PR, the configOption will have the fallback and deprecated keys. 
Like this
    
    ```java
        final ConfigOption<Integer> intConfig = ConfigOptions
            .key("rss.main.key")
            .intType()
            .defaultValue(100)
            .withFallbackKeys("rss.fallback.key1")
            .withDeprecatedKeys("rss.deprecated.key1");
    ```
    
    ### Does this PR introduce _any_ user-facing change?
    
    No.
    
    ### How was this patch tested?
    
    1. Existing UTs
    2. Newly UTs
---
 .../apache/uniffle/common/config/ConfigOption.java |  59 +++++++++++-
 .../apache/uniffle/common/config/FallbackKey.java  |  59 ++++++++++++
 .../org/apache/uniffle/common/config/RssConf.java  |  16 ++++
 .../uniffle/common/config/ConfigOptionTest.java    | 106 +++++++++++++++++++++
 4 files changed, 239 insertions(+), 1 deletion(-)

diff --git 
a/common/src/main/java/org/apache/uniffle/common/config/ConfigOption.java 
b/common/src/main/java/org/apache/uniffle/common/config/ConfigOption.java
index 7f420d71..fac73fbe 100644
--- a/common/src/main/java/org/apache/uniffle/common/config/ConfigOption.java
+++ b/common/src/main/java/org/apache/uniffle/common/config/ConfigOption.java
@@ -17,8 +17,11 @@
 
 package org.apache.uniffle.common.config;
 
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.Objects;
 import java.util.function.Function;
+import java.util.stream.Stream;
 
 /**
  * A {@code ConfigOption} describes a configuration parameter. It encapsulates
@@ -31,6 +34,7 @@ import java.util.function.Function;
  * @param <T> The type of value associated with the configuration option.
  */
 public class ConfigOption<T> {
+  static final FallbackKey[] EMPTY = new FallbackKey[0];
   static final String EMPTY_DESCRIPTION = "";
 
   /**
@@ -58,6 +62,9 @@ public class ConfigOption<T> {
    */
   private final Function<Object, T> converter;
 
+  /** The list of deprecated keys, in the order to be checked. */
+  private final FallbackKey[] fallbackKeys;
+
   /**
    * Creates a new config option with fallback keys.
    *
@@ -72,12 +79,14 @@ public class ConfigOption<T> {
       Class<?> clazz,
       String description,
       T defaultValue,
-      Function<Object, T> converter) {
+      Function<Object, T> converter,
+      FallbackKey... fallbackKeys) {
     this.key = Objects.requireNonNull(key);
     this.description = description;
     this.defaultValue = defaultValue;
     this.clazz = Objects.requireNonNull(clazz);
     this.converter = Objects.requireNonNull(converter);
+    this.fallbackKeys = fallbackKeys == null || fallbackKeys.length == 0 ? 
EMPTY : fallbackKeys;
   }
 
   /**
@@ -91,6 +100,45 @@ public class ConfigOption<T> {
     return new ConfigOption<>(key, clazz, description, defaultValue, 
converter);
   }
 
+  /**
+   * Creates a new config option, using this option's key and default value, 
and adding the given
+   * fallback keys.
+   *
+   * @param fallbackKeys The fallback keys, in the order in which they should 
be checked.
+   * @return A new config options, with the given fallback keys.
+   */
+  public ConfigOption<T> withFallbackKeys(String... fallbackKeys) {
+    final Stream<FallbackKey> newFallbackKeys =
+        Arrays.stream(fallbackKeys).map(FallbackKey::createFallbackKey);
+    final Stream<FallbackKey> currentAlternativeKeys = 
Arrays.stream(this.fallbackKeys);
+
+    // put fallback keys first so that they are prioritized
+    final FallbackKey[] mergedAlternativeKeys =
+        Stream.concat(newFallbackKeys, 
currentAlternativeKeys).toArray(FallbackKey[]::new);
+    return new ConfigOption<>(
+        key, clazz, description, defaultValue, converter, 
mergedAlternativeKeys);
+  }
+
+  /**
+   * Creates a new config option, using this option's key and default value, 
and adding the given
+   * deprecated keys.
+   *
+   * @param deprecatedKeys The deprecated keys, in the order in which they 
should be checked.
+   * @return A new config options, with the given deprecated keys.
+   */
+  public ConfigOption<T> withDeprecatedKeys(final String... deprecatedKeys) {
+    final Stream<FallbackKey> newDeprecatedKeys =
+        Arrays.stream(deprecatedKeys).map(FallbackKey::createDeprecatedKey);
+    final Stream<FallbackKey> currentAlternativeKeys = 
Arrays.stream(this.fallbackKeys);
+
+    // put deprecated keys last so that they are de-prioritized
+    final FallbackKey[] mergedAlternativeKeys =
+        Stream.concat(currentAlternativeKeys, newDeprecatedKeys)
+            .toArray(FallbackKey[]::new);
+    return new ConfigOption<>(
+        key, clazz, description, defaultValue, converter, 
mergedAlternativeKeys);
+  }
+
   // ------------------------------------------------------------------------
 
   /**
@@ -120,6 +168,15 @@ public class ConfigOption<T> {
     return defaultValue;
   }
 
+  /**
+   * Gets the fallback keys, in the order to be checked.
+   *
+   * @return The option's fallback keys.
+   */
+  public Iterable<FallbackKey> fallbackKeys() {
+    return (fallbackKeys == EMPTY) ? Collections.emptyList() : 
Arrays.asList(fallbackKeys);
+  }
+
   /**
    * Returns the class of value.
    *
diff --git 
a/common/src/main/java/org/apache/uniffle/common/config/FallbackKey.java 
b/common/src/main/java/org/apache/uniffle/common/config/FallbackKey.java
new file mode 100644
index 00000000..17a86115
--- /dev/null
+++ b/common/src/main/java/org/apache/uniffle/common/config/FallbackKey.java
@@ -0,0 +1,59 @@
+/*
+ * 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.uniffle.common.config;
+
+/** A key with FallbackKeys will fall back to the FallbackKeys if it itself is 
not configured. */
+public class FallbackKey {
+
+  // -------------------------
+  //  Factory methods
+  // -------------------------
+
+  static FallbackKey createFallbackKey(String key) {
+    return new FallbackKey(key, false);
+  }
+
+  static FallbackKey createDeprecatedKey(String key) {
+    return new FallbackKey(key, true);
+  }
+
+  // ------------------------------------------------------------------------
+
+  private final String key;
+
+  private final boolean isDeprecated;
+
+  public String getKey() {
+    return key;
+  }
+
+  public boolean isDeprecated() {
+    return isDeprecated;
+  }
+
+  private FallbackKey(String key, boolean isDeprecated) {
+    this.key = key;
+    this.isDeprecated = isDeprecated;
+  }
+
+  @Override
+  public String toString() {
+    return String.format("{key=%s, isDeprecated=%s}", key, isDeprecated);
+  }
+}
diff --git a/common/src/main/java/org/apache/uniffle/common/config/RssConf.java 
b/common/src/main/java/org/apache/uniffle/common/config/RssConf.java
index 1f9da666..d15e642f 100644
--- a/common/src/main/java/org/apache/uniffle/common/config/RssConf.java
+++ b/common/src/main/java/org/apache/uniffle/common/config/RssConf.java
@@ -18,6 +18,7 @@
 package org.apache.uniffle.common.config;
 
 import java.util.Arrays;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -569,8 +570,23 @@ public class RssConf implements Cloneable {
         .orElseGet(option::defaultValue);
   }
 
+  private Optional<Object> geRawValFromFallbackKeys(Iterator<FallbackKey> 
iter) {
+    if (iter.hasNext()) {
+      FallbackKey key = iter.next();
+      Optional<Object> rawVal = getRawValue(key.getKey());
+      if (!rawVal.isPresent()) {
+        return geRawValFromFallbackKeys(iter);
+      }
+      return rawVal;
+    }
+    return Optional.empty();
+  }
+
   public <T> Optional<T> getOptional(ConfigOption<T> option) {
     Optional<Object> rawValue = getRawValueFromOption(option);
+    if (!rawValue.isPresent()) {
+      rawValue = geRawValFromFallbackKeys(option.fallbackKeys().iterator());
+    }
     Class<?> clazz = option.getClazz();
     Optional<T> value = rawValue.map(v -> option.convertValue(v, clazz));
     return value;
diff --git 
a/common/src/test/java/org/apache/uniffle/common/config/ConfigOptionTest.java 
b/common/src/test/java/org/apache/uniffle/common/config/ConfigOptionTest.java
index 996fb6ea..7340715c 100644
--- 
a/common/src/test/java/org/apache/uniffle/common/config/ConfigOptionTest.java
+++ 
b/common/src/test/java/org/apache/uniffle/common/config/ConfigOptionTest.java
@@ -32,6 +32,112 @@ import static org.junit.jupiter.api.Assertions.fail;
 
 public class ConfigOptionTest {
 
+  @Test
+  public void testDeprecatedAndFallbackKeys() {
+    final ConfigOption<Integer> intConfig = ConfigOptions
+        .key("rss.main.key")
+        .intType().defaultValue(100)
+        .withFallbackKeys("rss.fallback.key1")
+        .withDeprecatedKeys("rss.deprecated.key1");
+
+    // case1
+    RssConf conf = new RssBaseConf();
+    assertEquals(100, conf.get(intConfig));
+
+    // case2
+    conf = new RssBaseConf();
+    conf.setString("rss.fallback.key1", "12");
+    conf.setString("rss.deprecated.key1", "13");
+    assertEquals(12, conf.get(intConfig));
+
+    // case3
+    conf = new RssBaseConf();
+    conf.setString("rss.deprecated.key1", "13");
+    assertEquals(13, conf.get(intConfig));
+
+    // case4
+    conf = new RssBaseConf();
+    conf.setString("rss.fallback.key1", "12");
+    assertEquals(12, conf.get(intConfig));
+
+    // case5
+    conf = new RssBaseConf();
+    conf.setString(intConfig.key(), "999");
+    conf.setString("rss.fallback.key1", "12");
+    conf.setString("rss.deprecated.key1", "13");
+    assertEquals(999, conf.get(intConfig));
+  }
+
+  @Test
+  public void testFallbackKeys() {
+    final ConfigOption<Integer> config1 = ConfigOptions
+        .key("rss.key.1")
+        .intType()
+        .defaultValue(1);
+
+    final ConfigOption<Integer> config2 = ConfigOptions
+        .key("rss.key.2")
+        .intType()
+        .defaultValue(2);
+
+    final ConfigOption<Integer> config3 = ConfigOptions
+        .key("rss.key.3")
+        .intType()
+        .defaultValue(9999)
+        .withFallbackKeys(config1.key(), config2.key());
+
+    // case1
+    RssConf conf = new RssBaseConf();
+    conf.setString(config1.key(), "10");
+    assertEquals(10, conf.get(config3));
+
+    // case2
+    conf.setString(config3.key(), "1111");
+    assertEquals(1111, conf.get(config3));
+
+    // case3
+    conf = new RssBaseConf();
+    assertEquals(9999, conf.get(config3));
+  }
+
+  @Test
+  public void testDeprecatedKeys() {
+    final ConfigOption<Integer> intConfig = ConfigOptions
+        .key("rss.key")
+        .intType()
+        .defaultValue(100)
+        .withDeprecatedKeys("rss.s1", "rss.s2", "rss.s3");
+
+    // case 1
+    RssConf conf = new RssBaseConf();
+    conf.setString("rss.s1", "10");
+    int val = conf.get(intConfig);
+    assertEquals(10, val);
+
+    // case 2
+    conf = new RssBaseConf();
+    conf.setString("rss.s1", "1");
+    conf.setString("rss.s2", "2");
+    conf.setString("rss.s3", "3");
+    assertEquals(1, conf.get(intConfig));
+
+    // case 3
+    conf = new RssBaseConf();
+    conf.setString("rss.s3", "3");
+    conf.setString("rss.s2", "2");
+    assertEquals(2, conf.get(intConfig));
+
+    // case 4
+    conf = new RssBaseConf();
+    conf.setString(intConfig.key(), "25");
+    conf.setString("rss.s3", "3");
+    assertEquals(25, conf.get(intConfig));
+
+    // case 5
+    conf = new RssBaseConf();
+    assertEquals(100, conf.get(intConfig));
+  }
+
   @Test
   public void testSetKVWithStringTypeDirectly() {
     final ConfigOption<Integer> intConfig = ConfigOptions

Reply via email to