Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support Discovery "format" specifier for google.protobuf.{Value,ListValue,Struct} #131

Merged
merged 18 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,12 @@ public enum Format {
UINT32("uint32"),
UINT64("uint64"),
FIXED32("fixed32"),
FIXED64("fixed64");
FIXED64("fixed64"),
// standard protobuf types:
ANY("google.protobuf.Any"),
LISTVALUE("google.protobuf.ListValue"),
STRUCT("google.protobuf.Struct"),
VALUE("google.protobuf.Value");

private String text;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.cloud.discotoproto3converter.disco.Method;
import com.google.cloud.discotoproto3converter.disco.Name;
import com.google.cloud.discotoproto3converter.disco.Schema;
import com.google.cloud.discotoproto3converter.disco.Schema.Format;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
Expand All @@ -46,6 +47,7 @@ public class DocumentToProtoConverter {
private final String relativeLinkPrefix;
private final boolean enumsAsStrings;
private boolean schemaRead;
private boolean usesStructProto;

// Set this to "true" to get some tracing output on stderr during development. Leave this as
// "false" for production code.
Expand All @@ -66,11 +68,14 @@ public DocumentToProtoConverter(
this.relativeLinkPrefix = relativeLinkPrefix;
this.protoFile.setMetadata(readDocumentMetadata(document, documentFileName));
this.enumsAsStrings = enumsAsStrings;
this.usesStructProto = false;

readSchema(document);
readResources(document);
cleanupEnumNamingConflicts();
this.protoFile.setHasLroDefinitions(applyLroConfiguration());
this.protoFile.setHasAnyFields(checkAnyFields());
this.protoFile.setUsesStructProto(this.usesStructProto);
convertEnumFieldsToStrings();
}

Expand All @@ -91,7 +96,7 @@ private ProtoFileMetadata readDocumentMetadata(Document document, String documen

private void readSchema(Document document) {
for (Map.Entry<String, Schema> entry : document.schemas().entrySet()) {
schemaToField(entry.getValue(), true, "*** readSchema\n");
schemaToField(entry.getValue(), true, "readSchema()");
}
for (Message message : protoFile.getMessages().values()) {
resolveReferences(message);
Expand Down Expand Up @@ -527,27 +532,55 @@ private Field schemaToField(Schema sch, boolean optional, String debugPreviousPa
Message valueType = null;
boolean repeated = false;
Message keyType = null;
String debugCurrentPath =
debugPreviousPath + String.format("SCHEMA: %s\n%s\n----\n", name, description);
String debugCurrentPath = String.format("%s.%s", debugPreviousPath, name);

if (trace) {
System.err.printf("*** schemaToField: \n%s", debugCurrentPath);
System.err.printf("*** schemaToField: %s\n", debugCurrentPath);
}

switch (sch.type()) {
case ANY:
valueType = Message.PRIMITIVES.get("google.protobuf.Any");
switch (sch.format()) {
case VALUE:
valueType = Message.PRIMITIVES.get("google.protobuf.Value");
this.usesStructProto = true;
break;
case EMPTY:
// intentional fall-through
case ANY:
valueType = Message.PRIMITIVES.get("google.protobuf.Any");
break;
default:
throw new IllegalStateException(
String.format(
"unexpected 'format' value ('%s') when processing ANY type in schema %s",
sch.format().toString(), debugCurrentPath));
}
break;
case ARRAY:
repeated = true;
if (sch.format() == Format.LISTVALUE) {
valueType = Message.PRIMITIVES.get("google.protobuf.ListValue");
this.usesStructProto = true;
// Since the `google.prootbuf.ListValue` whence this schema was generated is JSON-encoded
// as an array (see https://protobuf.dev/programming-guides/proto3/#json), the Discovery
// file describes the JSON array items. However, since we want to encode this schema back
// as an opaque `google.protobuf.ListValue` (which has the`repeated` semantics embedded
// internally), we should not make this field `repeated`.
} else {
repeated = true;
}
break;
case BOOLEAN:
valueType = Message.PRIMITIVES.get("bool");
break;
case EMPTY:
// This handles schemas with an "$ref" field
valueType = new Message(sch.reference(), true, false, null);
break;
case INTEGER:
switch (sch.format()) {
case EMPTY:
// intentional fall-through: if there's no format, we default to `int32`.
case INT32:
valueType = Message.PRIMITIVES.get("int32");
break;
Expand All @@ -566,24 +599,52 @@ private Field schemaToField(Schema sch, boolean optional, String debugPreviousPa
case FIXED64:
valueType = Message.PRIMITIVES.get("fixed64");
break;
default:
throw new IllegalStateException(
String.format(
"unexpected 'format' value ('%s') when processing INTEGER type in schema %s",
sch.format().toString(), debugCurrentPath));
}
break;
case NUMBER:
switch (sch.format()) {
case EMPTY:
// intentional fall-through: if there's no format, we default to `float`.
case FLOAT:
valueType = Message.PRIMITIVES.get("float");
break;
case DOUBLE:
valueType = Message.PRIMITIVES.get("double");
break;
default:
throw new IllegalStateException(
String.format(
"unexpected 'format' value ('%s') when processing NUMBER type in schema %s",
sch.format().toString(), debugCurrentPath));
}
break;
case OBJECT:
if (sch.additionalProperties() != null) {
repeated = true;
keyType = Message.PRIMITIVES.get("string");
} else {
valueType = new Message(getMessageName(sch), false, false, sanitizeDescr(description));
switch (sch.format()) {
case STRUCT:
valueType = Message.PRIMITIVES.get("google.protobuf.Struct");
this.usesStructProto = true;
// `additionalProperties' in the schema further specifies the JSON format, but
// "google.protobuf.Struct" is enough for specifying the proto message field type.
break;
case EMPTY:
if (sch.additionalProperties() != null) {
repeated = true;
keyType = Message.PRIMITIVES.get("string");
} else {
valueType =
new Message(getMessageName(sch), false, false, sanitizeDescr(description));
}
break;
default:
throw new IllegalStateException(
String.format(
"unexpected 'format' value ('%s') when processing OBJECT type in schema %s",
sch.format().toString(), debugCurrentPath));
}
break;
case STRING:
Expand All @@ -602,9 +663,26 @@ private Field schemaToField(Schema sch, boolean optional, String debugPreviousPa
case FIXED64:
valueType = Message.PRIMITIVES.get("fixed64");
break;
default:
case FLOAT:
valueType = Message.PRIMITIVES.get("float");
break;
case DOUBLE:
valueType = Message.PRIMITIVES.get("double");
break;
case BYTE:
// intentional fall-through for backwards compatibility. Ideally, we'd make this refer
// to the protobuf primitive type `byte`.
//
// TODO: use `byte` for new messages.
case EMPTY:
// If there's no format, we default to `string`.
valueType = Message.PRIMITIVES.get("string");
break;
default:
throw new IllegalStateException(
String.format(
"unexpected 'format' value ('%s') when processing STRING type in schema %s",
sch.format().toString(), debugCurrentPath));
}
}
break;
Expand All @@ -613,7 +691,9 @@ private Field schemaToField(Schema sch, boolean optional, String debugPreviousPa
if (repeated) {
Field subField =
schemaToField(
keyType == null ? sch.items() : sch.additionalProperties(), true, debugCurrentPath);
keyType == null ? sch.items() /* array */ : sch.additionalProperties() /* object */,
true,
debugCurrentPath);
valueType = subField.getValueType();
}

Expand Down Expand Up @@ -771,7 +851,7 @@ private void readResources(Document document) {

for (Schema pathParam : method.pathParams().values()) {
boolean required = methodSignatureParamNames.containsKey(pathParam.getIdentifier());
Field pathField = schemaToField(pathParam, !required, "readResources(A):) ");
Field pathField = schemaToField(pathParam, !required, "readResources(A)");
if (required) {
Option opt = createOption("google.api.field_behavior", ProtoOptionValues.REQUIRED);
pathField.getOptions().add(opt);
Expand All @@ -785,7 +865,7 @@ private void readResources(Document document) {

for (Schema queryParam : method.queryParams().values()) {
boolean required = methodSignatureParamNames.containsKey(queryParam.getIdentifier());
Field queryField = schemaToField(queryParam, !required, "readResources(B): ");
Field queryField = schemaToField(queryParam, !required, "readResources(B)");
if (required) {
Option opt = createOption("google.api.field_behavior", ProtoOptionValues.REQUIRED);
queryField.getOptions().add(opt);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,17 @@ public class Message extends ProtoElement<Message> {
PRIMITIVES.put("double", new Message("double", false, false, null));
PRIMITIVES.put("", new Message("", false, true, null));

// This isn't technically a primitive, but it is a fundamental well-known-type with no a priori
// structure.
// These aren't technically primitives, but they are opaque types we treat as such, essentially.
//
// TODO: If we start accepting additional well-known types, create a specific data structure for
// those rather than overloading "PRIMITIVES".
// TODO: Consider renaming this field more accurately, or creating a parallel field for these
// types.
PRIMITIVES.put("google.protobuf.Any", new Message("google.protobuf.Any", false, false, null));
PRIMITIVES.put(
"google.protobuf.Value", new Message("google.protobuf.Value", false, false, null));
PRIMITIVES.put(
"google.protobuf.ListValue", new Message("google.protobuf.ListValue", false, false, null));
PRIMITIVES.put(
"google.protobuf.Struct", new Message("google.protobuf.Struct", false, false, null));
}

private final SortedSet<Field> fields = new TreeSet<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,6 @@ public void writeToFile(PrintWriter writer, ProtoFile protoFile, boolean outputC

writer.println("package " + metadata.getProtoPkg() + ";\n");

// TODO: Place this import in the right alphabetical order. We are placing it here for now to
// work around an apparent bug in protobuf.js, where having this particular import be the last
// one makes the file not actually be imported.
if (protoFile.HasAnyFields()) {
writer.println("import \"google/protobuf/any.proto\";");
}

writer.println("import \"google/api/annotations.proto\";");
writer.println("import \"google/api/client.proto\";");
writer.println("import \"google/api/field_behavior.proto\";");
Expand All @@ -60,6 +53,14 @@ public void writeToFile(PrintWriter writer, ProtoFile protoFile, boolean outputC
if (protoFile.isHasLroDefinitions()) {
writer.println("import \"google/cloud/extended_operations.proto\";");
}

if (protoFile.HasAnyFields()) {
writer.println("import \"google/protobuf/any.proto\";");
}
if (protoFile.UsesStructProto()) {
writer.println("import \"google/protobuf/struct.proto\";");
}

writer.println();

// File Options
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public class ProtoFile {
private final Map<String, GrpcService> services = new TreeMap<>();
private boolean hasLroDefinitions;
private boolean hasAnyFields;
private boolean usesStructProto;

public ProtoFileMetadata getMetadata() {
return metadata;
Expand Down Expand Up @@ -56,4 +57,12 @@ public boolean HasAnyFields() {
public void setHasAnyFields(boolean hasAnyFields) {
this.hasAnyFields = hasAnyFields;
}

public boolean UsesStructProto() {
return usesStructProto;
}

public void setUsesStructProto(boolean usesStructProto) {
this.usesStructProto = usesStructProto;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public void convertVersioned() throws IOException {
"src", "test", "resources", prefix.toString(), "compute-versioned.proto.baseline");
String baselineBody = readFile(baselineFilePath);
System.out.printf(
"*** @Test:convertVersioned():\n*** Discovery path: %s\n*** Generated file: %s\n*** Baseline file: %s\n",
"*** @Test:convertVersioned():\n Discovery path: %s\n Generated file: %s\n Baseline file: %s\n",
discoveryDocPath.toAbsolutePath(),
generatedFilePath.toAbsolutePath(),
baselineFilePath.toAbsolutePath());
Expand Down Expand Up @@ -203,7 +203,7 @@ public void convertVersionedTwoServices() throws IOException {
"compute-versioned-two-services.proto.baseline");
String baselineBody = readFile(baselineFilePath);
System.out.printf(
"*** @Test:convertVersionedTwoServices():\n*** Discovery path: %s\n*** Generated file: %s\n*** Baseline file: %s\n",
"*** @Test:convertVersionedTwoServices():\n Discovery path: %s\n Generated file: %s\n Baseline file: %s\n",
discoveryDocPath.toAbsolutePath(),
generatedFilePath.toAbsolutePath(),
baselineFilePath.toAbsolutePath());
Expand Down Expand Up @@ -392,6 +392,13 @@ public void convertAnyFieldInError() throws IOException {
Paths.get(
"src", "test", "resources", prefix.toString(), "compute.error-any.proto.baseline");
String baselineBody = readFile(baselineFilePath);

System.out.printf(
"*** @Test:convertAnyFieldInError():\n Discovery path: %s\n Generated file: %s\n Baseline file: %s\n",
discoveryDocPath.toAbsolutePath(),
generatedFilePath.toAbsolutePath(),
baselineFilePath.toAbsolutePath());

assertEquals(baselineBody, actualBody);
}

Expand Down Expand Up @@ -523,6 +530,40 @@ public void convertWithMerge() throws IOException {
assertEquals(baselineBody, actualBody);
}

@Test
public void convertAnyFieldWithFormat() throws IOException {
DiscoToProto3ConverterApp app = new DiscoToProto3ConverterApp();
Path prefix = Paths.get("google", "cloud", "compute", "v1small");
Path discoveryDocPath =
Paths.get("src", "test", "resources", prefix.toString(), "compute.v1small.any-format.json");
Path generatedFilePath =
Paths.get(outputDir.toString(), prefix.toString(), "compute.any-format.proto");

app.convert(
discoveryDocPath.toString(),
null,
generatedFilePath.toString(),
"",
"",
"https://cloud.google.com",
"true",
"true");

String actualBody = readFile(generatedFilePath);
Path baselineFilePath =
Paths.get(
"src", "test", "resources", prefix.toString(), "compute.any-format.proto.baseline");
String baselineBody = readFile(baselineFilePath);

System.out.printf(
"*** @Test:convertAnyFieldWithFormat():\n Discovery path: %s\n Generated file: %s\n Baseline file: %s\n",
discoveryDocPath.toAbsolutePath(),
generatedFilePath.toAbsolutePath(),
baselineFilePath.toAbsolutePath());

assertEquals(baselineBody, actualBody);
}

private static String readFile(Path path) throws IOException {
return new String(Files.readAllBytes(path), StandardCharsets.UTF_8);
}
Expand Down
Loading