This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 95812fbd37 [#13244] fix(common): fall back to the default for
present-but-blank config values (#13245)
95812fbd37 is described below
commit 95812fbd37314f1460f04053208c890270f38fdf
Author: YangJie <[email protected]>
AuthorDate: Mon Sep 21 04:22:36 2026 -0400
[#13244] fix(common): fall back to the default for present-but-blank config
values (#13245)
### What changes were proposed in this pull request?
After conversion, `readFrom` now returns the `defaultValue` when the
converted result is `null` and a non-null default exists, matching the
absent-key path. String entries and no-default entries are unchanged.
### Why are the changes needed?
`readFrom` only consulted the default for an absent key, so a
present-but-blank value converted to `null` for the typed converters and
bypassed the default, making every typed consumer NPE on unboxing.
Fix: #13244
### Does this PR introduce _any_ user-facing change?
No API change. A present-but-blank config value now falls back to its
configured default for int/long/double/boolean entries instead of
returning `null`. String entries (where blank is valid) and entries with
no default are unchanged.
### How was this patch tested?
Added assertions in `TestConfigEntry`, which pin that a
present-but-blank value falls back to the non-null default for the typed
converters; they fail on the pre-fix tree and pass after the fix.
---
.../org/apache/gravitino/config/ConfigEntry.java | 5 +++++
.../org/apache/gravitino/config/TestConfigEntry.java | 20 ++++++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/common/src/main/java/org/apache/gravitino/config/ConfigEntry.java
b/common/src/main/java/org/apache/gravitino/config/ConfigEntry.java
index d8d704a1c8..62c44da1cb 100644
--- a/common/src/main/java/org/apache/gravitino/config/ConfigEntry.java
+++ b/common/src/main/java/org/apache/gravitino/config/ConfigEntry.java
@@ -316,6 +316,11 @@ public class ConfigEntry<T> {
}
T convertedValue = valueConverter.apply(value);
+ if (convertedValue == null && defaultValue != null) {
+ // A present-but-blank value is converted to null by the typed
converters (int/long/double/
+ // boolean); fall back to the configured default instead of returning
null.
+ return defaultValue;
+ }
if (validator != null) {
validator.accept(convertedValue);
}
diff --git
a/core/src/test/java/org/apache/gravitino/config/TestConfigEntry.java
b/core/src/test/java/org/apache/gravitino/config/TestConfigEntry.java
index 2973e437db..3a29aa8942 100644
--- a/core/src/test/java/org/apache/gravitino/config/TestConfigEntry.java
+++ b/core/src/test/java/org/apache/gravitino/config/TestConfigEntry.java
@@ -70,6 +70,26 @@ public class TestConfigEntry {
new
ConfigBuilder("gravitino.test.boolean").booleanConf().createWithDefault(true);
boolean value2 = testConf2.readFrom(configMap);
Assertions.assertTrue(value2);
+
+ // A present-but-blank value must not bypass the default: typed converters
map blank to null,
+ // and before the fix readFrom returned that null instead of the
configured default.
+ configMap.put("gravitino.test.int.blank", " ");
+ ConfigEntry<Integer> blankIntConf =
+ new ConfigBuilder("gravitino.test.int.blank")
+ .doc("test")
+ .version("1.0")
+ .intConf()
+ .createWithDefault(10);
+ Assertions.assertEquals(10, blankIntConf.readFrom(configMap));
+
+ configMap.put("gravitino.test.long.blank", " ");
+ ConfigEntry<Long> blankLongConf =
+ new ConfigBuilder("gravitino.test.long.blank")
+ .doc("test")
+ .version("1.0")
+ .longConf()
+ .createWithDefault(20L);
+ Assertions.assertEquals(20L, blankLongConf.readFrom(configMap));
}
@Test
