googleapis / googleapis/google-http-java-client

GenericData.containsKey() returns true for unset (null) declared fields, violating Map contract

Open Beginner friendly
#2,187 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.