You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@flink.apache.org by fa...@apache.org on 2023/12/29 02:11:38 UTC
(flink) branch release-1.17 updated (e4b83bb2740 -> 8de5beedc33)
This is an automated email from the ASF dual-hosted git repository.
fanrui pushed a change to branch release-1.17
in repository https://gitbox.apache.org/repos/asf/flink.git
from e4b83bb2740 [docs][hotfix] Set available Flink Pulsar connector for 1.17 to v4.1
new 34ff5422df6 [FLINK-33942][configuration][junit5-migration] Migrate DelegatingConfigurationTest to Junit5 and Assertj
new ac7c7fa1ab9 [FLINK-33942][configuration] Fix the bug that DelegatingConfiguration misses the prefix in some get methods
new 8de5beedc33 [FLINK-33942][configuration][refactor] Using ConfigOption instead of string key in DelegatingConfiguration
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../configuration/DelegatingConfiguration.java | 22 ++---
.../configuration/DelegatingConfigurationTest.java | 94 ++++++++++++++++------
2 files changed, 80 insertions(+), 36 deletions(-)
(flink) 03/03: [FLINK-33942][configuration][refactor] Using ConfigOption instead of string key in DelegatingConfiguration
Posted by fa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
fanrui pushed a commit to branch release-1.17
in repository https://gitbox.apache.org/repos/asf/flink.git
commit 8de5beedc330f8e1f2ce9fd5ae719b828c8cb6c6
Author: Rui Fan <19...@gmail.com>
AuthorDate: Tue Dec 26 18:46:44 2023 +0800
[FLINK-33942][configuration][refactor] Using ConfigOption instead of string key in DelegatingConfiguration
---
.../apache/flink/configuration/DelegatingConfiguration.java | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/flink-core/src/main/java/org/apache/flink/configuration/DelegatingConfiguration.java b/flink-core/src/main/java/org/apache/flink/configuration/DelegatingConfiguration.java
index 67150b714f4..b9eaad709a1 100644
--- a/flink-core/src/main/java/org/apache/flink/configuration/DelegatingConfiguration.java
+++ b/flink-core/src/main/java/org/apache/flink/configuration/DelegatingConfiguration.java
@@ -90,7 +90,7 @@ public final class DelegatingConfiguration extends Configuration {
@Override
public void setString(ConfigOption<String> key, String value) {
- this.backingConfig.setString(prefix + key.key(), value);
+ this.backingConfig.setString(prefixOption(key, prefix), value);
}
@Override
@@ -127,7 +127,7 @@ public final class DelegatingConfiguration extends Configuration {
@Override
public void setInteger(ConfigOption<Integer> key, int value) {
- this.backingConfig.setInteger(prefix + key.key(), value);
+ this.backingConfig.setInteger(prefixOption(key, prefix), value);
}
@Override
@@ -152,7 +152,7 @@ public final class DelegatingConfiguration extends Configuration {
@Override
public void setLong(ConfigOption<Long> key, long value) {
- this.backingConfig.setLong(prefix + key.key(), value);
+ this.backingConfig.setLong(prefixOption(key, prefix), value);
}
@Override
@@ -172,7 +172,7 @@ public final class DelegatingConfiguration extends Configuration {
@Override
public void setBoolean(ConfigOption<Boolean> key, boolean value) {
- this.backingConfig.setBoolean(prefix + key.key(), value);
+ this.backingConfig.setBoolean(prefixOption(key, prefix), value);
}
@Override
@@ -202,7 +202,7 @@ public final class DelegatingConfiguration extends Configuration {
@Override
public void setFloat(ConfigOption<Float> key, float value) {
- this.backingConfig.setFloat(prefix + key.key(), value);
+ this.backingConfig.setFloat(prefixOption(key, prefix), value);
}
@Override
@@ -227,7 +227,7 @@ public final class DelegatingConfiguration extends Configuration {
@Override
public void setDouble(ConfigOption<Double> key, double value) {
- this.backingConfig.setDouble(prefix + key.key(), value);
+ this.backingConfig.setDouble(prefixOption(key, prefix), value);
}
@Override
(flink) 02/03: [FLINK-33942][configuration] Fix the bug that DelegatingConfiguration misses the prefix in some get methods
Posted by fa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
fanrui pushed a commit to branch release-1.17
in repository https://gitbox.apache.org/repos/asf/flink.git
commit ac7c7fa1ab900ed59260bb49750a1bc2b9f1d200
Author: Rui Fan <19...@gmail.com>
AuthorDate: Tue Dec 26 17:08:07 2023 +0800
[FLINK-33942][configuration] Fix the bug that DelegatingConfiguration misses the prefix in some get methods
---
.../configuration/DelegatingConfiguration.java | 10 ++---
.../configuration/DelegatingConfigurationTest.java | 50 ++++++++++++++++++++++
2 files changed, 55 insertions(+), 5 deletions(-)
diff --git a/flink-core/src/main/java/org/apache/flink/configuration/DelegatingConfiguration.java b/flink-core/src/main/java/org/apache/flink/configuration/DelegatingConfiguration.java
index 573e75d86d4..67150b714f4 100644
--- a/flink-core/src/main/java/org/apache/flink/configuration/DelegatingConfiguration.java
+++ b/flink-core/src/main/java/org/apache/flink/configuration/DelegatingConfiguration.java
@@ -117,7 +117,7 @@ public final class DelegatingConfiguration extends Configuration {
@Override
public int getInteger(ConfigOption<Integer> configOption, int overrideDefault) {
- return this.backingConfig.getInteger(configOption, overrideDefault);
+ return this.backingConfig.getInteger(prefixOption(configOption, prefix), overrideDefault);
}
@Override
@@ -142,7 +142,7 @@ public final class DelegatingConfiguration extends Configuration {
@Override
public long getLong(ConfigOption<Long> configOption, long overrideDefault) {
- return this.backingConfig.getLong(configOption, overrideDefault);
+ return this.backingConfig.getLong(prefixOption(configOption, prefix), overrideDefault);
}
@Override
@@ -177,7 +177,7 @@ public final class DelegatingConfiguration extends Configuration {
@Override
public boolean getBoolean(ConfigOption<Boolean> configOption, boolean overrideDefault) {
- return this.backingConfig.getBoolean(configOption, overrideDefault);
+ return this.backingConfig.getBoolean(prefixOption(configOption, prefix), overrideDefault);
}
@Override
@@ -192,7 +192,7 @@ public final class DelegatingConfiguration extends Configuration {
@Override
public float getFloat(ConfigOption<Float> configOption, float overrideDefault) {
- return this.backingConfig.getFloat(configOption, overrideDefault);
+ return this.backingConfig.getFloat(prefixOption(configOption, prefix), overrideDefault);
}
@Override
@@ -217,7 +217,7 @@ public final class DelegatingConfiguration extends Configuration {
@Override
public double getDouble(ConfigOption<Double> configOption, double overrideDefault) {
- return this.backingConfig.getDouble(configOption, overrideDefault);
+ return this.backingConfig.getDouble(prefixOption(configOption, prefix), overrideDefault);
}
@Override
diff --git a/flink-core/src/test/java/org/apache/flink/configuration/DelegatingConfigurationTest.java b/flink-core/src/test/java/org/apache/flink/configuration/DelegatingConfigurationTest.java
index cec52629806..59cb4718f9c 100644
--- a/flink-core/src/test/java/org/apache/flink/configuration/DelegatingConfigurationTest.java
+++ b/flink-core/src/test/java/org/apache/flink/configuration/DelegatingConfigurationTest.java
@@ -141,4 +141,54 @@ class DelegatingConfigurationTest {
assertThat(delegatingConf.set(CoreOptions.DEFAULT_PARALLELISM, 1)).isSameAs(delegatingConf);
}
+
+ @Test
+ void testGetWithOverrideDefault() {
+ Configuration original = new Configuration();
+ final DelegatingConfiguration delegatingConf =
+ new DelegatingConfiguration(original, "prefix.");
+
+ // Test for integer
+ ConfigOption<Integer> integerOption =
+ ConfigOptions.key("integer.key").intType().noDefaultValue();
+
+ // integerOption doesn't exist in delegatingConf, and it should be overrideDefault.
+ original.setInteger(integerOption, 1);
+ assertThat(delegatingConf.getInteger(integerOption, 2)).isEqualTo(2);
+
+ // integerOption exists in delegatingConf, and it should be value that set before.
+ delegatingConf.setInteger(integerOption, 3);
+ assertThat(delegatingConf.getInteger(integerOption, 2)).isEqualTo(3);
+
+ // Test for float
+ ConfigOption<Float> floatOption =
+ ConfigOptions.key("float.key").floatType().noDefaultValue();
+ original.setFloat(floatOption, 4f);
+ assertThat(delegatingConf.getFloat(floatOption, 5f)).isEqualTo(5f);
+ delegatingConf.setFloat(floatOption, 6f);
+ assertThat(delegatingConf.getFloat(floatOption, 5f)).isEqualTo(6f);
+
+ // Test for double
+ ConfigOption<Double> doubleOption =
+ ConfigOptions.key("double.key").doubleType().noDefaultValue();
+ original.setDouble(doubleOption, 7d);
+ assertThat(delegatingConf.getDouble(doubleOption, 8d)).isEqualTo(8d);
+ delegatingConf.setDouble(doubleOption, 9f);
+ assertThat(delegatingConf.getDouble(doubleOption, 8d)).isEqualTo(9f);
+
+ // Test for long
+ ConfigOption<Long> longOption = ConfigOptions.key("long.key").longType().noDefaultValue();
+ original.setLong(longOption, 10L);
+ assertThat(delegatingConf.getLong(longOption, 11L)).isEqualTo(11L);
+ delegatingConf.setLong(longOption, 12L);
+ assertThat(delegatingConf.getLong(longOption, 11L)).isEqualTo(12L);
+
+ // Test for boolean
+ ConfigOption<Boolean> booleanOption =
+ ConfigOptions.key("boolean.key").booleanType().noDefaultValue();
+ original.setBoolean(booleanOption, false);
+ assertThat(delegatingConf.getBoolean(booleanOption, true)).isEqualTo(true);
+ delegatingConf.setBoolean(booleanOption, false);
+ assertThat(delegatingConf.getBoolean(booleanOption, true)).isEqualTo(false);
+ }
}
(flink) 01/03: [FLINK-33942][configuration][junit5-migration] Migrate DelegatingConfigurationTest to Junit5 and Assertj
Posted by fa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
fanrui pushed a commit to branch release-1.17
in repository https://gitbox.apache.org/repos/asf/flink.git
commit 34ff5422df68701beb3ed8edf5589caf26c4447d
Author: Rui Fan <19...@gmail.com>
AuthorDate: Tue Dec 26 17:06:20 2023 +0800
[FLINK-33942][configuration][junit5-migration] Migrate DelegatingConfigurationTest to Junit5 and Assertj
---
.../configuration/DelegatingConfigurationTest.java | 44 ++++++++++------------
1 file changed, 19 insertions(+), 25 deletions(-)
diff --git a/flink-core/src/test/java/org/apache/flink/configuration/DelegatingConfigurationTest.java b/flink-core/src/test/java/org/apache/flink/configuration/DelegatingConfigurationTest.java
index bbd4692a13a..cec52629806 100644
--- a/flink-core/src/test/java/org/apache/flink/configuration/DelegatingConfigurationTest.java
+++ b/flink-core/src/test/java/org/apache/flink/configuration/DelegatingConfigurationTest.java
@@ -18,8 +18,7 @@
package org.apache.flink.configuration;
-import org.assertj.core.api.Assertions;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
@@ -27,14 +26,13 @@ import java.util.Map;
import java.util.Properties;
import java.util.Set;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
/** Tests for the {@link DelegatingConfiguration}. */
-public class DelegatingConfigurationTest {
+class DelegatingConfigurationTest {
@Test
- public void testIfDelegatesImplementAllMethods() throws IllegalArgumentException {
+ void testIfDelegatesImplementAllMethods() throws IllegalArgumentException {
// For each method in the Configuration class...
Method[] confMethods = Configuration.class.getDeclaredMethods();
@@ -70,27 +68,28 @@ public class DelegatingConfigurationTest {
}
}
- assertTrue(
- "Configuration method '"
- + configurationMethod.getName()
- + "' has not been wrapped correctly in DelegatingConfiguration wrapper",
- hasMethod);
+ assertThat(hasMethod)
+ .as(
+ "Configuration method '"
+ + configurationMethod.getName()
+ + "' has not been wrapped correctly in DelegatingConfiguration wrapper")
+ .isTrue();
}
}
@Test
- public void testDelegationConfigurationWithNullPrefix() {
+ void testDelegationConfigurationWithNullPrefix() {
Configuration backingConf = new Configuration();
backingConf.setValueInternal("test-key", "value", false);
DelegatingConfiguration configuration = new DelegatingConfiguration(backingConf, null);
Set<String> keySet = configuration.keySet();
- assertEquals(keySet, backingConf.keySet());
+ assertThat(backingConf.keySet()).isEqualTo(keySet);
}
@Test
- public void testDelegationConfigurationWithPrefix() {
+ void testDelegationConfigurationWithPrefix() {
String prefix = "pref-";
String expectedKey = "key";
@@ -102,9 +101,7 @@ public class DelegatingConfigurationTest {
DelegatingConfiguration configuration = new DelegatingConfiguration(backingConf, prefix);
Set<String> keySet = configuration.keySet();
-
- assertEquals(keySet.size(), 1);
- assertEquals(keySet.iterator().next(), expectedKey);
+ assertThat(keySet).hasSize(1).containsExactly(expectedKey);
/*
* Key does not match the prefix
@@ -113,13 +110,11 @@ public class DelegatingConfigurationTest {
backingConf.setValueInternal("test-key", "value", false);
configuration = new DelegatingConfiguration(backingConf, prefix);
- keySet = configuration.keySet();
-
- assertTrue(keySet.isEmpty());
+ assertThat(configuration.keySet()).isEmpty();
}
@Test
- public void testDelegationConfigurationToMapConsistentWithAddAllToProperties() {
+ void testDelegationConfigurationToMapConsistentWithAddAllToProperties() {
Configuration conf = new Configuration();
conf.setString("k0", "v0");
conf.setString("prefix.k1", "v1");
@@ -136,15 +131,14 @@ public class DelegatingConfigurationTest {
mapProperties.put(entry.getKey(), entry.getValue());
}
// Verification
- assertEquals(properties, mapProperties);
+ assertThat(mapProperties).isEqualTo(properties);
}
@Test
- public void testSetReturnsDelegatingConfiguration() {
+ void testSetReturnsDelegatingConfiguration() {
final Configuration conf = new Configuration();
final DelegatingConfiguration delegatingConf = new DelegatingConfiguration(conf, "prefix.");
- Assertions.assertThat(delegatingConf.set(CoreOptions.DEFAULT_PARALLELISM, 1))
- .isSameAs(delegatingConf);
+ assertThat(delegatingConf.set(CoreOptions.DEFAULT_PARALLELISM, 1)).isSameAs(delegatingConf);
}
}