Skip to content

[common] Fix NumberFormatException when parsing empty string properties #2372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sushantmane wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[common] Fix NumberFormatException when parsing empty string properties #2372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bdd8d4c
[common] Fix NumberFormatException when parsing empty string properties
sushantmane Jan 13, 2026
42dabaa
Address PR review comments: fix whitespace handling and add tests
sushantmane Jan 13, 2026
0ef4ee9
Optimize trim() calls to once per method invocation
sushantmane Jan 13, 2026
861f7e5
Address review comments
sushantmane Jan 13, 2026
a119b64
Fix flaky tests
sushantmane Jan 14, 2026
c2180c0
Optimize containsKey checks to reduce redundant map lookups
sushantmane Jan 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 61 additions & 49 deletions internal/venice-client-common/src/main/java/com/linkedin/venice/utils/VeniceProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,95 +339,107 @@ public boolean getBoolean(String key) {
}
}

public long getLong(String name, long defaultValue) {
if (containsKey(name)) {
return Long.parseLong(get(name));
} else {
return defaultValue;
/**
* Helper method to get a trimmed, non-empty string value or return null.
* This centralizes the null and empty string handling logic for numeric parsing methods.
*
* @param name The property name
* @return The trimmed value if it's non-null and non-empty, null otherwise
*/
private String getTrimmedValueOrNull(String name) {
String value = props.get(name);
// Value can be null if key doesn't exist or if explicitly stored as null in the underlying map
if (value == null || value.trim().isEmpty()) {
return null;
}
return value.trim();
}

public long getLong(String name, long defaultValue) {
String trimmedValue = getTrimmedValueOrNull(name);
return trimmedValue != null ? Long.parseLong(trimmedValue) : defaultValue;
}

public long getLong(String name) {
if (containsKey(name)) {
return Long.parseLong(get(name));
} else {
throw new UndefinedPropertyException(name);
String trimmedValue = getTrimmedValueOrNull(name);
if (trimmedValue == null) {
Copy link
Contributor

@arjun4084346 arjun4084346 Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for minor comments; but i think L365-L368 can also go inside method getTrimmedValueOrNull and it can be renamed like getTrimmedNonEmptyValueOrThrowException

if (!containsKey(name)) {
throw new UndefinedPropertyException(name);
}
throw new VeniceException("Property " + name + " is defined but has an empty value");
}
return Long.parseLong(trimmedValue);
}

public int getInt(String name, int defaultValue) {
if (containsKey(name)) {
return Integer.parseInt(get(name));
} else {
return defaultValue;
}
String trimmedValue = getTrimmedValueOrNull(name);
return trimmedValue != null ? Integer.parseInt(trimmedValue) : defaultValue;
}

public int getInt(String name) {
if (containsKey(name)) {
return Integer.parseInt(get(name));
} else {
throw new UndefinedPropertyException(name);
String trimmedValue = getTrimmedValueOrNull(name);
if (trimmedValue == null) {
if (!containsKey(name)) {
throw new UndefinedPropertyException(name);
}
throw new VeniceException("Property " + name + " is defined but has an empty value");
}
return Integer.parseInt(trimmedValue);
}

public Optional<Integer> getOptionalInt(String name) {
if (containsKey(name)) {
return Optional.of(Integer.parseInt(get(name)));
} else {
return Optional.empty();
}
String trimmedValue = getTrimmedValueOrNull(name);
return trimmedValue != null ? Optional.of(Integer.parseInt(trimmedValue)) : Optional.empty();
}

public double getDouble(String name, double defaultValue) {
if (containsKey(name)) {
return Double.parseDouble(get(name));
} else {
return defaultValue;
}
String trimmedValue = getTrimmedValueOrNull(name);
return trimmedValue != null ? Double.parseDouble(trimmedValue) : defaultValue;
}

public double getDouble(String name) {
if (containsKey(name)) {
return Double.parseDouble(get(name));
} else {
throw new UndefinedPropertyException(name);
String trimmedValue = getTrimmedValueOrNull(name);
if (trimmedValue == null) {
if (!containsKey(name)) {
throw new UndefinedPropertyException(name);
}
throw new VeniceException("Property " + name + " is defined but has an empty value");
}
return Double.parseDouble(trimmedValue);
}

public long getSizeInBytes(String name, long defaultValue) {
if (containsKey(name)) {
return getSizeInBytes(name);
} else {
return defaultValue;
}
String trimmedValue = getTrimmedValueOrNull(name);
return trimmedValue != null ? convertSizeFromLiteral(trimmedValue) : defaultValue;
}

public long getSizeInBytes(String name) {
if (!containsKey(name)) {
throw new UndefinedPropertyException(name);
String trimmedValue = getTrimmedValueOrNull(name);
if (trimmedValue == null) {
if (!containsKey(name)) {
throw new UndefinedPropertyException(name);
}
throw new VeniceException("Property " + name + " is defined but has an empty value");
}

String bytes = get(name);
return convertSizeFromLiteral(bytes);
return convertSizeFromLiteral(trimmedValue);
}

public static long convertSizeFromLiteral(String size) {
String sizeLc = size.toLowerCase().trim();
if (sizeLc.endsWith("kb")) {
return Long.parseLong(size.substring(0, size.length() - 2)) * 1024;
return Long.parseLong(sizeLc.substring(0, sizeLc.length() - 2)) * 1024;
} else if (sizeLc.endsWith("k")) {
return Long.parseLong(size.substring(0, size.length() - 1)) * 1024;
return Long.parseLong(sizeLc.substring(0, sizeLc.length() - 1)) * 1024;
} else if (sizeLc.endsWith("mb")) {
return Long.parseLong(size.substring(0, size.length() - 2)) * 1024 * 1024;
return Long.parseLong(sizeLc.substring(0, sizeLc.length() - 2)) * 1024 * 1024;
} else if (sizeLc.endsWith("m")) {
return Long.parseLong(size.substring(0, size.length() - 1)) * 1024 * 1024;
return Long.parseLong(sizeLc.substring(0, sizeLc.length() - 1)) * 1024 * 1024;
} else if (sizeLc.endsWith("gb")) {
return Long.parseLong(size.substring(0, size.length() - 2)) * 1024 * 1024 * 1024;
return Long.parseLong(sizeLc.substring(0, sizeLc.length() - 2)) * 1024 * 1024 * 1024;
} else if (sizeLc.endsWith("g")) {
return Long.parseLong(size.substring(0, size.length() - 1)) * 1024 * 1024 * 1024;
return Long.parseLong(sizeLc.substring(0, sizeLc.length() - 1)) * 1024 * 1024 * 1024;
} else {
return Long.parseLong(size);
return Long.parseLong(sizeLc);
}
}

Expand Down
Loading
Loading