googleapis / googleapis/google-cloud-java

ProtoSchemaConverter produces an unresolvable schema when an enum is referenced by multiple non-top-level messages

Abierto
#13,786 0 comentarios 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
Java
Estrellas
2.1k
Forks
1.2k
Merge medio
1 d 23 h
PR fusionados (30 d)
154

Descripción

> **Prior art:** [java-bigquerystorage#428](https://github.com/googleapis/java-bigquerystorage/pull/428) (*"fix: ProtoSchemaConverter's problem when converting fields reference same enum"*, merged 2020-07-20) fixed the case where **multiple fields** reference the same enum, by introducing the global `enumTypes` dedup set and the synthetic `_E` wrapper. That fix is incomplete: the single wrapper is emitted into the scope of the *first* message that references the enum, so it only resolves for references reachable from there (same message, or a top-level reference). It does **not** cover the same enum being referenced from **multiple sibling nested messages** when none of them is the top-level message — the case below. No follow-up issue was filed for that gap.

## Summary

`ProtoSchemaConverter.convert(Descriptor)` generates a self-contained `DescriptorProto` that the backend cannot rebuild when the **same enum is referenced from more than one message and none of those references is on the top-level message**. The resulting write stream fails with:

```
failed to create file descriptor: proto: message field "…Inner2.color" cannot resolve type: "*.example_Color_E.Color" not found
```

and the client retries indefinitely.

## Root cause

In `ProtoSchemaConverter.convertInternal`, message types are hoisted to the **root** of the schema (`rootProtoSchema.addNestedType(...)`), but enums are handled differently: an enum is wrapped in a synthetic enclosing struct `_E` that is added to **`resultProto`** — the *local* scope of whichever message is currently being converted — and this is guarded by the **global** `enumTypes` dedup set introduced in #428, so the wrapper is emitted only once, for the first message that references the enum.

```java
if (inputField.getType() == FieldDescriptor.Type.ENUM) {
String enumFullName = inputField.getEnumType().getFullName();
String enclosingTypeName = getNameFromFullName(enumFullName) + "_E";
String enumName = inputField.getEnumType().getName();
String actualEnumFullName = enclosingTypeName + "." + enumName;
if (enumTypes.contains(enumFullName)) {
resultField.setTypeName(actualEnumFullName); // (B) subsequent messages: reference only
} else {
EnumDescriptorProto enumType = inputField.getEnumType().toProto();
resultProto.addNestedType( // (A) wrapper added to LOCAL scope, not root
DescriptorProto.newBuilder()
.setName(enclosingTypeName)
.addEnumType(enumType.toBuilder().setName(enumName))
.build());
resultField.setTypeName(actualEnumFullName);
enumTypes.add(enumFullName);
}
}
```

Consequently:

- The **first** message to reference the enum gets the `_E` wrapper nested inside its own scope, so its `type_name` (`_E.`, a *relative* name) resolves.
- **Every other** message gets the same relative `type_name` but no wrapper in scope. Proto name resolution walks the referencing message's scope outward to the root and never finds the wrapper — it lives inside a *sibling* message. Resolution fails.

This only manifests when no reference is on the top-level message, because for the top-level message `resultProto == rootProtoSchema`, so a wrapper created there does land at the root and is visible to everyone.

## Minimal reproduction

`example.proto`:

```proto
syntax = "proto3";

package example;

enum Color {
COLOR_UNSPECIFIED = 0;
RED = 1;
GREEN = 2;
}

message Inner1 {
Color color = 1;
}

message Inner2 {
Color color = 1;
}

message Root {
Inner1 inner1 = 1;
Inner2 inner2 = 2;
}
```

```java
ProtoSchema schema = ProtoSchemaConverter.convert(Root.getDescriptor());

// Rebuild it the way the backend does:
FileDescriptorProto file = FileDescriptorProto.newBuilder()
.setName("__schema.proto")
.addMessageType(schema.getProtoDescriptor())
.build();
Descriptors.FileDescriptor.buildFrom(file, new Descriptors.FileDescriptor[] {});
```

## Actual result

The converter emits the `example_Color_E` wrapper only inside `example_Inner1`; `example_Inner2.color` points at a type that is not in its scope:

```
name: "example_Root"
field { name: "inner1" type: TYPE_MESSAGE type_name: "example_Inner1" }
field { name: "inner2" type: TYPE_MESSAGE type_name: "example_Inner2" }
nested_type {
name: "example_Inner1"
field { name: "color" type: TYPE_ENUM type_name: "example_Color_E.Color" }
nested_type { # <-- wrapper lives ONLY here
name: "example_Color_E"
enum_type { name: "Color" value {...} }
}
}
nested_type {
name: "example_Inner2"
field { name: "color" type: TYPE_ENUM type_name: "example_Color_E.Color" } # <-- unresolvable
}
```

`buildFrom(...)` throws:

```
example_Root.example_Inner2.color: "example_Color_E.Color" is not defined.
```

The real Storage Write API surfaces the same thing as a retriable `UNKNOWN` on the write stream (`failed to create file descriptor … not found`).

## Expected result

The flattened schema should be resolvable regardless of how many messages reference the enum, or where. Emitting the `_E` wrapper once at the **root** (as a sibling of the hoisted messages, matching how message types are already handled) fixes it: the relative `type_name` then resolves from any message's scope.

## Workaround

Post-process the `ProtoSchema` and relocate every `_E` enum wrapper to the root `DescriptorProto` before calling `setWriterSchema(...)`.

## Environment

- `com.google.cloud:google-cloud-bigquerystorage` 3.28.0; reproduced identically against 3.29.0.
- The enum-handling block in `ProtoSchemaConverter` is unchanged since #428 (2020) and is identical in the current `google-cloud-java` monorepo copy.

Guía de contribución

Abrir la guía de contribución

Línea de trabajo

Start at ProtoSchemaConverter.convertInternal and the enum-handling block described in the issue; reproduce the case with example.proto and rebuild the descriptor with Descriptors.FileDescriptor.buildFrom(...). Done when the synthetic enum wrapper is emitted at the root and the schema resolves for references from multiple nested messages.

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

Evaluación

Stack tecnológico
google-cloud, java
Área
api, backend
Tipo de issue
Error
Dificultad
3/5
Tiempo estimado
1-2 días
Estado de actividad
Tranquilo
Claridad
Bastante claro
Aptitud para principiantes
55/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.