From 1fc00a3e630c2c2d160fa6459f55f19e85fa327c Mon Sep 17 00:00:00 2001 From: baoloongmao Date: Tue, 1 Sep 2020 08:34:59 +0800 Subject: [PATCH 1/7] HDDS-4104. Provide a way to get the default value and key of java-based-configuration easily --- .../conf/ConfigurationReflectionUtil.java | 50 +++++++++++++ .../conf/TestConfigurationReflectionUtil.java | 71 +++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java diff --git a/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java b/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java index 48f05491893d..5e03b1b03ee8 100644 --- a/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java +++ b/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java @@ -23,6 +23,8 @@ import java.lang.reflect.Modifier; import java.util.Deque; import java.util.LinkedList; +import java.util.Optional; +import java.util.stream.Stream; /** * Reflection utilities for configuration injection. @@ -240,4 +242,52 @@ private static void updateConfigurationFromObject( } } } + + public static Optional getDefaultValue(Class configClass, + String fieldName) { + Config annotation = findFieldConfigAnnotationByName(configClass, + fieldName); + if (annotation != null) { + return Optional.of(annotation.defaultValue()); + } + return Optional.empty(); + } + + public static Optional getKey(Class configClass, + String fieldName) { + ConfigGroup configGroup = + configClass.getAnnotation(ConfigGroup.class); + + Config annotation = findFieldConfigAnnotationByName(configClass, + fieldName); + if (annotation != null) { + String key = annotation.key(); + if (configGroup != null) { + key = configGroup.prefix() + "." + annotation.key(); + } + return Optional.of(key); + } + return Optional.empty(); + } + + public static Optional getType(Class configClass, + String fieldName) { + Config config = findFieldConfigAnnotationByName(configClass, + fieldName); + if (config != null) { + return Optional.of(config.type()); + } + return Optional.empty(); + } + + private static Config findFieldConfigAnnotationByName(Class configClass, + String fieldName) { + Optional field = Stream.of(configClass.getDeclaredFields()) + .filter(f -> f.getName().equals(fieldName)) + .findFirst(); + if (field.isPresent()) { + return field.get().getAnnotation(Config.class); + } + return null; + } } diff --git a/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java b/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java new file mode 100644 index 000000000000..2e3ffe793e52 --- /dev/null +++ b/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java @@ -0,0 +1,71 @@ +/** + * 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.hdds.conf; + +import org.junit.Assert; +import org.junit.Test; + +import java.util.Optional; + +/** + * Test the configuration reflection utility class. + */ +public class TestConfigurationReflectionUtil { + + @Test + public void testClassWithConfigGroup() { + Optional actualType = + ConfigurationReflectionUtil.getType( + ConfigurationExample.class, "waitTime"); + Assert.assertTrue(actualType.isPresent()); + Assert.assertEquals(ConfigType.TIME, actualType.get()); + + Optional actualKey = + ConfigurationReflectionUtil.getKey( + ConfigurationExample.class, "waitTime"); + Assert.assertTrue(actualKey.isPresent()); + Assert.assertEquals("ozone.scm.client.wait", actualKey.get()); + + Optional actualDefaultValue = + ConfigurationReflectionUtil.getDefaultValue( + ConfigurationExample.class, "waitTime"); + Assert.assertTrue(actualKey.isPresent()); + Assert.assertEquals("30m", actualDefaultValue.get()); + } + + @Test + public void testClassWithoutConfigGroup() { + Optional actualType = + ConfigurationReflectionUtil.getType( + ConfigurationExampleGrandParent.class, "number"); + Assert.assertTrue(actualType.isPresent()); + Assert.assertEquals(ConfigType.AUTO, actualType.get()); + + Optional actualKey = + ConfigurationReflectionUtil.getKey( + ConfigurationExampleGrandParent.class, "number"); + Assert.assertTrue(actualKey.isPresent()); + Assert.assertEquals("number", actualKey.get()); + + Optional actualDefaultValue = + ConfigurationReflectionUtil.getDefaultValue( + ConfigurationExampleGrandParent.class, "number"); + Assert.assertTrue(actualKey.isPresent()); + Assert.assertEquals("2", actualDefaultValue.get()); + } +} From 747c92510b5052dfdec0559c5977d785d6eb6dc1 Mon Sep 17 00:00:00 2001 From: maobaolong <307499405@qq.com> Date: Wed, 2 Sep 2020 15:42:04 +0800 Subject: [PATCH 2/7] Apply suggestions from code review Co-authored-by: Doroszlai, Attila <6454655+adoroszlai@users.noreply.github.com> --- .../hadoop/hdds/conf/TestConfigurationReflectionUtil.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java b/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java index 2e3ffe793e52..d5c3ce7b72c7 100644 --- a/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java +++ b/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java @@ -44,7 +44,7 @@ public void testClassWithConfigGroup() { Optional actualDefaultValue = ConfigurationReflectionUtil.getDefaultValue( ConfigurationExample.class, "waitTime"); - Assert.assertTrue(actualKey.isPresent()); + Assert.assertTrue(actualDefaultValue.isPresent()); Assert.assertEquals("30m", actualDefaultValue.get()); } @@ -65,7 +65,7 @@ public void testClassWithoutConfigGroup() { Optional actualDefaultValue = ConfigurationReflectionUtil.getDefaultValue( ConfigurationExampleGrandParent.class, "number"); - Assert.assertTrue(actualKey.isPresent()); + Assert.assertTrue(actualDefaultValue.isPresent()); Assert.assertEquals("2", actualDefaultValue.get()); } } From 957cb5f48564ee512d91f32724bfed6db91221d1 Mon Sep 17 00:00:00 2001 From: baoloongmao Date: Wed, 2 Sep 2020 20:12:23 +0800 Subject: [PATCH 3/7] add tests. --- .../conf/ConfigurationReflectionUtil.java | 57 ++++++----- .../conf/TestConfigurationReflectionUtil.java | 94 +++++++++++++------ .../dev-support/intellij/ozone-site.xml | 16 ++-- 3 files changed, 101 insertions(+), 66 deletions(-) diff --git a/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java b/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java index 5e03b1b03ee8..89de4d341fb1 100644 --- a/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java +++ b/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java @@ -245,12 +245,8 @@ private static void updateConfigurationFromObject( public static Optional getDefaultValue(Class configClass, String fieldName) { - Config annotation = findFieldConfigAnnotationByName(configClass, - fieldName); - if (annotation != null) { - return Optional.of(annotation.defaultValue()); - } - return Optional.empty(); + return findFieldConfigAnnotationByName(configClass, fieldName) + .map(Config::defaultValue); } public static Optional getKey(Class configClass, @@ -258,36 +254,37 @@ public static Optional getKey(Class configClass, ConfigGroup configGroup = configClass.getAnnotation(ConfigGroup.class); - Config annotation = findFieldConfigAnnotationByName(configClass, - fieldName); - if (annotation != null) { - String key = annotation.key(); - if (configGroup != null) { - key = configGroup.prefix() + "." + annotation.key(); - } - return Optional.of(key); - } - return Optional.empty(); + return findFieldConfigAnnotationByName(configClass, + fieldName).map( + config -> configGroup == null ? config.key() + : configGroup.prefix() + "." + config.key()); } public static Optional getType(Class configClass, String fieldName) { - Config config = findFieldConfigAnnotationByName(configClass, - fieldName); - if (config != null) { - return Optional.of(config.type()); - } - return Optional.empty(); + return findFieldConfigAnnotationByName(configClass, fieldName) + .map(Config::type); } - private static Config findFieldConfigAnnotationByName(Class configClass, - String fieldName) { - Optional field = Stream.of(configClass.getDeclaredFields()) - .filter(f -> f.getName().equals(fieldName)) - .findFirst(); - if (field.isPresent()) { - return field.get().getAnnotation(Config.class); + private static Optional findFieldConfigAnnotationByName( + final Class configClass, String fieldName) { + Class theClass = configClass; + while (theClass != null) { + Optional config = Stream.of(theClass.getDeclaredFields()) + .filter(f -> f.getName().equals(fieldName)) + .findFirst() + .map(f -> f.getAnnotation(Config.class)); + + if (config.isPresent()) { + return config; + } + + if (!theClass.getSuperclass().equals(Object.class)) { + theClass = theClass.getSuperclass(); + } else { + theClass = null; + } } - return null; + return Optional.empty(); } } diff --git a/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java b/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java index d5c3ce7b72c7..e94f72a7afca 100644 --- a/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java +++ b/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java @@ -19,53 +19,91 @@ import org.junit.Assert; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import java.util.Arrays; +import java.util.Collection; import java.util.Optional; /** * Test the configuration reflection utility class. */ +@RunWith(Parameterized.class) public class TestConfigurationReflectionUtil { - @Test - public void testClassWithConfigGroup() { - Optional actualType = - ConfigurationReflectionUtil.getType( - ConfigurationExample.class, "waitTime"); - Assert.assertTrue(actualType.isPresent()); - Assert.assertEquals(ConfigType.TIME, actualType.get()); + private final ConfigType type; + private final String key; + private final String defaultValue; + private Class testClass; + private String fieldName; + private boolean typePresent; + private boolean keyPresent; + private boolean defaultValuePresent; - Optional actualKey = - ConfigurationReflectionUtil.getKey( - ConfigurationExample.class, "waitTime"); - Assert.assertTrue(actualKey.isPresent()); - Assert.assertEquals("ozone.scm.client.wait", actualKey.get()); + @Parameterized.Parameters + public static Collection data() { + return Arrays.asList(new Object[][]{ + {ConfigurationExample.class, "waitTime", + ConfigType.TIME, true, + "ozone.scm.client.wait", true, + "30m", true}, + {ConfigurationExampleGrandParent.class, "number", + ConfigType.AUTO, true, + "number", true, + "2", true}, + {ConfigurationExample.class, "secure", + ConfigType.AUTO, true, + "ozone.scm.client.secure", true, + "true", true}, + {ConfigurationExample.class, "no-such-field", + null, false, + "", false, + "", false}, + {ConfigFileAppender.class, "document", + null, false, + "", false, + "", false}, + }); + } - Optional actualDefaultValue = - ConfigurationReflectionUtil.getDefaultValue( - ConfigurationExample.class, "waitTime"); - Assert.assertTrue(actualDefaultValue.isPresent()); - Assert.assertEquals("30m", actualDefaultValue.get()); + public TestConfigurationReflectionUtil(Class testClass, String fieldName, + ConfigType type, boolean typePresent, + String key, boolean keyPresent, + String defaultValue, boolean defaultValuePresent) { + this.testClass = testClass; + this.fieldName = fieldName; + this.typePresent = typePresent; + this.type = type; + this.key = key; + this.keyPresent = keyPresent; + this.defaultValue = defaultValue; + this.defaultValuePresent = defaultValuePresent; } @Test - public void testClassWithoutConfigGroup() { + public void testClassWithConfigGroup() { Optional actualType = ConfigurationReflectionUtil.getType( - ConfigurationExampleGrandParent.class, "number"); - Assert.assertTrue(actualType.isPresent()); - Assert.assertEquals(ConfigType.AUTO, actualType.get()); + testClass, fieldName); + Assert.assertEquals(typePresent, actualType.isPresent()); + if (typePresent) { + Assert.assertEquals(type, actualType.get()); + } Optional actualKey = ConfigurationReflectionUtil.getKey( - ConfigurationExampleGrandParent.class, "number"); - Assert.assertTrue(actualKey.isPresent()); - Assert.assertEquals("number", actualKey.get()); - + testClass, fieldName); + Assert.assertEquals(keyPresent, actualKey.isPresent()); + if (keyPresent) { + Assert.assertEquals(key, actualKey.get()); + } Optional actualDefaultValue = ConfigurationReflectionUtil.getDefaultValue( - ConfigurationExampleGrandParent.class, "number"); - Assert.assertTrue(actualDefaultValue.isPresent()); - Assert.assertEquals("2", actualDefaultValue.get()); + testClass, fieldName); + Assert.assertEquals(defaultValuePresent, actualDefaultValue.isPresent()); + if (defaultValuePresent) { + Assert.assertEquals(defaultValue, actualDefaultValue.get()); + } } } diff --git a/hadoop-ozone/dev-support/intellij/ozone-site.xml b/hadoop-ozone/dev-support/intellij/ozone-site.xml index 3fde85065e74..3006048cf999 100644 --- a/hadoop-ozone/dev-support/intellij/ozone-site.xml +++ b/hadoop-ozone/dev-support/intellij/ozone-site.xml @@ -59,12 +59,12 @@ hdds.prometheus.endpoint.enabled true - - ozone.recon.address - localhost:9891 - - - ozone.recon.db.dir - /tmp/recon - + + + + + + + + From 67c37ebfaf77aef946752b517086d1451ab7c8cd Mon Sep 17 00:00:00 2001 From: baoloongmao Date: Thu, 3 Sep 2020 00:34:52 +0800 Subject: [PATCH 4/7] Fix style --- .../hdds/conf/TestConfigurationReflectionUtil.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java b/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java index e94f72a7afca..b4d416e8324f 100644 --- a/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java +++ b/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java @@ -35,11 +35,11 @@ public class TestConfigurationReflectionUtil { private final ConfigType type; private final String key; private final String defaultValue; - private Class testClass; - private String fieldName; - private boolean typePresent; - private boolean keyPresent; - private boolean defaultValuePresent; + private final Class testClass; + private final String fieldName; + private final boolean typePresent; + private final boolean keyPresent; + private final boolean defaultValuePresent; @Parameterized.Parameters public static Collection data() { @@ -67,7 +67,9 @@ public static Collection data() { }); } - public TestConfigurationReflectionUtil(Class testClass, String fieldName, + @SuppressWarnings("checkstyle:ParameterNumber") + public TestConfigurationReflectionUtil( + Class testClass, String fieldName, ConfigType type, boolean typePresent, String key, boolean keyPresent, String defaultValue, boolean defaultValuePresent) { From e409d54ec852aea4399a12626f0c99fbc3a5e36f Mon Sep 17 00:00:00 2001 From: baoloongmao Date: Thu, 3 Sep 2020 07:34:52 +0800 Subject: [PATCH 5/7] revert ozone-site.xml and rename testcase. --- .../conf/TestConfigurationReflectionUtil.java | 2 +- hadoop-ozone/dev-support/intellij/ozone-site.xml | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java b/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java index b4d416e8324f..7f523c39030f 100644 --- a/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java +++ b/hadoop-hdds/config/src/test/java/org/apache/hadoop/hdds/conf/TestConfigurationReflectionUtil.java @@ -84,7 +84,7 @@ public TestConfigurationReflectionUtil( } @Test - public void testClassWithConfigGroup() { + public void testForGivenClasses() { Optional actualType = ConfigurationReflectionUtil.getType( testClass, fieldName); diff --git a/hadoop-ozone/dev-support/intellij/ozone-site.xml b/hadoop-ozone/dev-support/intellij/ozone-site.xml index 3006048cf999..3fde85065e74 100644 --- a/hadoop-ozone/dev-support/intellij/ozone-site.xml +++ b/hadoop-ozone/dev-support/intellij/ozone-site.xml @@ -59,12 +59,12 @@ hdds.prometheus.endpoint.enabled true - - - - - - - - + + ozone.recon.address + localhost:9891 + + + ozone.recon.db.dir + /tmp/recon + From a497618d21b7bd95540be4c644d91c327cc82d85 Mon Sep 17 00:00:00 2001 From: maobaolong <307499405@qq.com> Date: Thu, 3 Sep 2020 23:59:16 +0800 Subject: [PATCH 6/7] Update hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java Co-authored-by: Doroszlai, Attila <6454655+adoroszlai@users.noreply.github.com> --- .../apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java b/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java index 89de4d341fb1..0cd4260c5422 100644 --- a/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java +++ b/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java @@ -279,9 +279,8 @@ private static Optional findFieldConfigAnnotationByName( return config; } - if (!theClass.getSuperclass().equals(Object.class)) { - theClass = theClass.getSuperclass(); - } else { + theClass = theClass.getSuperclass(); + if (Object.class.equals(theClass)) { theClass = null; } } From dc210f5e72efaf5e533d8992b5397901de24af46 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Thu, 3 Sep 2020 22:08:36 +0200 Subject: [PATCH 7/7] trigger new CI check