googleapis / googleapis/google-http-java-client

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

Abierto Apto para principiantes
#2,187 1 comentario 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
Java
Estrellas
1.4k
Forks
473
Métricas de merge de PR
Sin PR fusionados en 30 d

Descripción

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

Guía de contribución

Abrir la guía de contribución

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
java
Área
developer-experience
Tipo de issue
Error
Dificultad
2/5
Tiempo estimado
1-3 horas
Estado de actividad
Activo
Claridad
Bien especificado
Aptitud para principiantes
84/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.