googleapis / googleapis/google-http-java-client

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

Ouverte Adaptée aux débutants
#2,187 1 commentaire 0 réactions 0 personnes assignées Voir sur GitHub
Langage dominant
Java
Étoiles
1.4k
Forks
473
Métriques de merge des PR
Aucune PR mergée en 30 j

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);
}
```

Guide de contribution

Ouvrir le guide de contribution

Piste de recherche

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.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
java
Domaine
developer-experience
Type d'issue
Bug
Difficulté
2/5
Temps estimé
1-3 heures
Activité
Active
Clarté
Clairement spécifiée
Accessibilité débutants
84/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.