googleapis / googleapis/google-http-java-client
GenericData.containsKey() returns true for unset (null) declared fields, violating Map contract
- Dominant language
- Java
- Stars
- 1.4k
- Forks
- 473
- PR merge metrics
- No merged PRs in 30d
Description
### Description
PR #2151 introduced an override for `GenericData.containsKey(Object name)` that queries `classInfo.hasFieldInfo(fieldName)`. However, `ClassInfo.hasFieldInfo` checks only whether the `@Key` field is declared in class reflection metadata, rather than checking whether the field holds a non-null value on the instance.
### Impact & Broken Invariants
In `GenericData`, declared fields with `null` values are treated as absent from the map:
1. **`keySet()` / `entrySet()` Contradiction**:
On `new MyData()`, `containsKey("field")` returns `true`, but `get("field")` is `null`, `entrySet()` has size `0`, and `keySet().toString()` outputs `[]`.
2. **`Set.contains` vs `Iterator` Inconsistency**:
Because `java.util.AbstractMap.keySet().contains(k)` delegates to `Map.containsKey(k)`, `model.keySet().contains("field")` evaluates to `true`, while iterating over `model.keySet()` yields `0` elements.
3. **Client Breakages**:
Code patterns checking for field presence (such as pagination checks like `if (response.containsKey("pageToken"))`) now evaluate to `true` even when the server never populated the field.
### Reproduction
```java
public class ExampleModel extends GenericData {
@Key private String optionalField;
}
ExampleModel model = new ExampleModel();
// Prior to 2.2.0:
// model.containsKey("optionalField") == false
// In 2.2.0:
model.containsKey("optionalField"); // returns true!
model.get("optionalField"); // returns null
model.keySet(); // prints []
model.keySet().contains("optionalField"); // returns true while iterator is empty
```
### Proposed Fix
In `com.google.api.client.util.GenericData.java`, check whether the declared field value is non-null, matching `DataMap.containsKey()`:
```java
@Override
public final boolean containsKey(Object name) {
if (!(name instanceof String)) {
return false;
}
String fieldName = (String) name;
FieldInfo fieldInfo = classInfo.getFieldInfo(fieldName);
if (fieldInfo != null) {
return fieldInfo.getValue(this) != null;
}
if (classInfo.getIgnoreCase()) {
fieldName = fieldName.toLowerCase(Locale.US);
}
return unknownFields.containsKey(fieldName);
}
```
Contributor guide
Research direction
Start in com.google.api.client.util.GenericData.java and inspect containsKey alongside DataMap.containsKey and the referenced FieldInfo access. Verify the ExampleModel reproduction, then confirm declared null fields are absent while non-null declared and unknown fields retain the expected Map behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- developer-experience
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 84/100