googleapis/disco-to-proto3-converter

Google Discovery Document to Proto3 Converter

Quick Start

Requirements

To build and run this tool you will need the following tools:

• JDK 8+
• Maven 3.3.9+

Build

To build the standalone jar run the following command from the repository root:

mvn package

Test

To run the unit tests execute the following command:

mvn test

Run

After performing the build, to run the converter using compute.v1.json as a sample input (included in this repository) run the following command from the repository root:

java \
  -jar target/disco-to-proto3-converter-0.0.1-SNAPSHOT-jar-with-dependencies.jar \
  --discovery_doc_path=src/test/resources/google/cloud/compute/v1/compute.v1.json \
  --output_file_path=google/cloud/compute/v1/compute.proto \
  --enums_as_strings=True

Check the google/cloud/compute/v1 directory for the converted compute.proto file.

Bazel

The converter can also be used from Bazel via the proto_from_disco bazel rule like the following:

load(
    "@com_google_disco_to_proto3_converter//rules_gapic:disco_to_proto.bzl",
    "proto_from_disco",
)

proto_from_disco(
    name = "compute",
    src = "//:src/test/resources/compute.v1.json",
)

This is not an officially supported Google product

disco-to-proto3-converter's Issues

merging protos created from the same source should be a no-op

In the course of working on #112 , I discovered that merging a synthetic proto with itself, which one would expect to be a no-op, actually changes the protos.

Two ways to see this;

1. In src/test/java/com/google/cloud/discotoproto3converter/DiscoToProto3ConverterAppTest.java, re-enable the assertEquals in convertWithIdempotentMerge and examine the test failure. Some fields get extra comments.
2. Use the reproduction steps in #113, which is related

Build fails on Windows

Environment details

• Programming language: Java
• OS: Windows 11
• Language runtime version: Oracle JDK 17, with MVN 3.8.4

Steps to reproduce

1. Git clone the repo
2. Launch "mvn package" in it

Here are the tests that fails :

Results :

Failed tests:   convertWithIgnorelist(com.google.cloud.discotoproto3converter.DiscoToProto3ConverterAppTest): expected:<(..)ght 2020 Google LLC[
  convertEnumsAsStrings(com.google.cloud.discotoproto3converter.DiscoToProto3ConverterAppTest): expected:<...ight 2020 G(..)e LLC[
(..)nvert(com.google.cloud.discotoproto3converter.DiscoToProto3ConverterAppTest): expected:<...ight 2020 Google LLC[
(..)nvert(com.google.cloud.discotoproto3converter.GapicYamlGeneratorAppTest): expected:<...ight 2020 Google LLC[

Tests run: 5, Failures: 4, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  1.563 s
[INFO] Finished at: 2021-12-07T00:52:13+01:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.12.4:test (default-test) on project disco-to-proto3-converter: There are test failures.
[ERROR]
[ERROR] Please refer to C:\Users\thher\disco-to-proto3-converter\target\surefire-reports for the individual test results.
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException

I can confirm that it works on Linux, with OpenJDK.

Errors with type ANY, and some well-formated camel cases

Environment details

• Programming language: Java
• OS: Debian 10
• Language runtime version: OpenJDK Runtime Environment (build 13.0.5.1+1-Debian-1)

Steps to reproduce

1. Download an example Discovery Document here : https://people-pa.clients6.google.com/$discovery/rest
2. Do java -jar disco-to-proto3-converter-0.0.1-SNAPSHOT-jar-with-dependencies.jar --discovery_doc_path=people-pa-v2.json --output_file_path=people-pa-v2.proto

I had 2 main errors :

1. The first one is that it doesn't like the type "ANY", it doesn't seems very popular, another discovery documents parser had the same error.. so as a workaround I changed it to a string, just to see if it pass
   The full error :

❯ java -jar disco-to-proto3-converter-0.0.1-SNAPSHOT-jar-with-dependencies.jar --discovery_doc_path=people-pa-v2.json --output_file_path=people-pa-v2.proto
Exception in thread "main" java.lang.IllegalArgumentException: Any type detected in schema: Schema "", type ANY
	at com.google.cloud.discotoproto3converter.proto3.DocumentToProtoConverter.schemaToField(DocumentToProtoConverter.java:409)
	at com.google.cloud.discotoproto3converter.proto3.DocumentToProtoConverter.schemaToField(DocumentToProtoConverter.java:484)
	at com.google.cloud.discotoproto3converter.proto3.DocumentToProtoConverter.schemaToField(DocumentToProtoConverter.java:484)
	at com.google.cloud.discotoproto3converter.proto3.DocumentToProtoConverter.schemaToField(DocumentToProtoConverter.java:496)
	at com.google.cloud.discotoproto3converter.proto3.DocumentToProtoConverter.readSchema(DocumentToProtoConverter.java:101)
	at com.google.cloud.discotoproto3converter.proto3.DocumentToProtoConverter.<init>(DocumentToProtoConverter.java:65)
	at com.google.cloud.discotoproto3converter.ConverterApp.convert(ConverterApp.java:62)
	at com.google.cloud.discotoproto3converter.ConverterApp.convert(ConverterApp.java:75)
	at com.google.cloud.discotoproto3converter.DiscoToProto3ConverterApp.main(DiscoToProto3ConverterApp.java:29)

2. Then, it doesn't like the "people_pa" because it's not in camel case, understable we can change it even if it's from official APIs :

Exception in thread "main" java.lang.IllegalArgumentException: Name: identifier not in NO_CHECK camel: 'people_pa'
	at com.google.cloud.discotoproto3converter.disco.Name.validateCamel(Name.java:110)
	at com.google.cloud.discotoproto3converter.disco.Name.camelInternal(Name.java:76)
	at com.google.cloud.discotoproto3converter.disco.Name.anyCamel(Name.java:58)
	at com.google.cloud.discotoproto3converter.proto3.DocumentToProtoConverter.getRpcMessageName(DocumentToProtoConverter.java:747)
	at com.google.cloud.discotoproto3converter.proto3.DocumentToProtoConverter.readResources(DocumentToProtoConverter.java:613)
	at com.google.cloud.discotoproto3converter.proto3.DocumentToProtoConverter.<init>(DocumentToProtoConverter.java:66)
	at com.google.cloud.discotoproto3converter.ConverterApp.convert(ConverterApp.java:62)
	at com.google.cloud.discotoproto3converter.ConverterApp.convert(ConverterApp.java:75)
	at com.google.cloud.discotoproto3converter.DiscoToProto3ConverterApp.main(DiscoToProto3ConverterApp.java:29)

But then, it gives the same error on "context.consistencyOptions.levelOptions.exactTimestamp" and a lot of other ones, each time it has at least one dot in it:

Exception in thread "main" java.lang.IllegalArgumentException: Name: identifier not in NO_CHECK camel: 'context.consistencyOptions.levelOptions.exactTimestamp'
	at com.google.cloud.discotoproto3converter.disco.Name.validateCamel(Name.java:110)
	at com.google.cloud.discotoproto3converter.disco.Name.camelInternal(Name.java:76)
	at com.google.cloud.discotoproto3converter.disco.Name.anyCamel(Name.java:58)
	at com.google.cloud.discotoproto3converter.proto3.DocumentToProtoConverter.schemaToField(DocumentToProtoConverter.java:401)
	at com.google.cloud.discotoproto3converter.proto3.DocumentToProtoConverter.readResources(DocumentToProtoConverter.java:642)
	at com.google.cloud.discotoproto3converter.proto3.DocumentToProtoConverter.<init>(DocumentToProtoConverter.java:66)
	at com.google.cloud.discotoproto3converter.ConverterApp.convert(ConverterApp.java:62)
	at com.google.cloud.discotoproto3converter.ConverterApp.convert(ConverterApp.java:75)
	at com.google.cloud.discotoproto3converter.DiscoToProto3ConverterApp.main(DiscoToProto3ConverterApp.java:29)

It only pass if I remove all the dots from the concerned identifier !

Audit generated comment format

As of #4

• proto field comments imported from Discovery docs contain markup like // (== resource_for {$api_version}.addresses ==) (== resource_for {$api_version}.globalAddresses ==) Consider processing the comments to make this more human-friendly, or possibly removing it altogether if not needed.
• lines aren't wrapped at 80 chars like as per the external protocol buffer style guide. We should consider doing that.

Also note that there are open discussions about whether we'll need to append other information to proto comments (eg default values), so ideally we'll consider all these comment manipulations together.

We should do this before a beta release of the converter or downstream GAPICs.

consolidate code: parseMethods + ParseResources

parseResources and parseMethods appear to have essentially identical code that only differs in what is returned. For clarity and to reduce duplication, consider combining the two.

One idea is to pass in a lambda to a common function that can be used to call resources.put or methods.addAll; the latter can be the default if no lambda is provided, since it is the recursive case.

Enable master branch protection

      This repository does not seem to have master branch
      protection enabled, at least in the way I'm expecting.
      I was hoping for:

      - master branch protection
      - requiring at least one code reviewer
      - requiring at least two status checks
      - enforcing rules for admins

      Please turn it on!

merging protos with Any fields generates a fictional, invalid proto field

So far I've only seen this in generated proto messages that have a field of type protobuf.Any. If one merges such a proto file with another one, a new fictional field is created in the message containing the Any. The fictional field is is listed inside the .proto file as null google = XXX, where XXX is a field number. What's even stranger is that this happens even if the "previous" .proto being merged with came from the exact same source as the current proto being generated.

Reproduction steps:

1. Disable the check in copyField() introduced in #112:

    if (valueType == null && keyType == null) {      
      // return null;    <-- disable this line
   

2. Generate the proto file /tmp/compute.examine.0.proto without merging with any previous proto (no --previous_proto_file_path):

   java -jar target/disco-to-proto3-converter-0.0.1-SNAPSHOT-jar-with-dependencies.jar "--discovery_doc_path=${DIREGAPIC_GOOGLEAPIS}/google/cloud/compute/v1/compute.v1.json" '--service_ignorelist=HttpHealthChecks,HttpsHealthChecks' '--message_ignorelist=HttpHealthCheck,HttpsHealthCheck,HttpHealthCheckList,HttpsHealthCheckList,GetHttpHealthCheckRequest,GetHttpsHealthCheckRequest,PatchHttpHealthCheckRequest,PatchHttpsHealthCheckRequest,UpdateHttpHealthCheckRequest,UpdateHttpsHealthCheckRequest,InsertHttpHealthCheckRequest,InsertHttpsHealthCheckRequest,ListHttpHealthChecksRequest,ListHttpsHealthChecksRequest,DeleteHttpHealthCheckRequest,DeleteHttpsHealthCheckRequest' '--enums_as_strings=True' '--output_file_path=/tmp/compute.examine.0.proto'
   

3. Generate the proto file /tmp/compute.examine.1.proto from the same source, but merging with the previous /tmp/compute.examine.0.proto (with the --previous_proto_file_path):

   java -jar target/disco-to-proto3-converter-0.0.1-SNAPSHOT-jar-with-dependencies.jar "--discovery_doc_path=${DIREGAPIC_GOOGLEAPIS}/google/cloud/compute/v1/compute.v1.json" "--previous_proto_file_path=/tmp/compute.examine.0.proto" '--service_ignorelist=HttpHealthChecks,HttpsHealthChecks' '--message_ignorelist=HttpHealthCheck,HttpsHealthCheck,HttpHealthCheckList,HttpsHealthCheckList,GetHttpHealthCheckRequest,GetHttpsHealthCheckRequest,PatchHttpHealthCheckRequest,PatchHttpsHealthCheckRequest,UpdateHttpHealthCheckRequest,UpdateHttpsHealthCheckRequest,InsertHttpHealthCheckRequest,InsertHttpsHealthCheckRequest,ListHttpHealthChecksRequest,ListHttpsHealthChecksRequest,DeleteHttpHealthCheckRequest,DeleteHttpsHealthCheckRequest' '--enums_as_strings=True' '--output_file_path=/tmp/compute.examine.1.proto'
   

4. Diff /tmp/compute.examine.0.proto and /tmp/compute.examine.0.proto

Dependency Dashboard

This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

Edited/Blocked

These updates have been manually edited so Renovate will no longer make changes. To discard all commits and start over, click on a checkbox.

• fix(deps): update dependency com.fasterxml.jackson.core:jackson-annotations to v2.17.2
• chore(deps): update dependency org.apache.maven.plugins:maven-assembly-plugin to v3.7.1

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

• fix(deps): update dependency com.fasterxml.jackson.core:jackson-core to v2.17.2
• fix(deps): update dependency com.fasterxml.jackson.core:jackson-databind to v2.17.2
• fix(deps): update dependency com.google.auto.value:auto-value to v1.11.0
• fix(deps): update dependency com.google.auto.value:auto-value-annotations to v1.11.0
• fix(deps): update dependency com.google.code.gson:gson to v2.11.0
• fix(deps): update dependency com.google.guava:guava to v33.2.1-jre
• Click on this checkbox to rebase all open PRs at once

Detected dependencies

github-actions

.github/workflows/ci.yaml

• actions/checkout v4
• actions/cache v4
• gcr.io/gapic-images/googleapis-bazel 20210105

maven

pom.xml

• com.google.auto.value:auto-value 1.10.4
• com.google.auto.value:auto-value-annotations 1.10.4
• com.fasterxml.jackson.core:jackson-annotations 2.17.0
• com.fasterxml.jackson.core:jackson-core 2.17.0
• com.fasterxml.jackson.core:jackson-databind 2.17.0
• com.google.guava:guava 33.1.0-jre
• com.google.code.gson:gson 2.10.1
• javax.annotation:javax.annotation-api 1.3.2
• com.google.code.findbugs:jsr305 3.0.2
• junit:junit 4.13.2
• org.apache.maven.plugins:maven-assembly-plugin 3.6.0

---

• Check this box to trigger a request for Renovate to run again on this repository

Dependency Dashboard

This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

Rate-Limited

These updates are currently rate-limited. Click on a checkbox below to force their creation now.

• fix(deps): update dependency com.fasterxml.jackson.core:jackson-annotations to v2.14.2
• fix(deps): update dependency com.fasterxml.jackson.core:jackson-core to v2.14.2
• fix(deps): update dependency com.google.auto.value:auto-value to v1.10.1
• fix(deps): update dependency com.google.auto.value:auto-value-annotations to v1.10.1
• fix(deps): update dependency com.google.code.gson:gson to v2.10.1
• chore(deps): update actions/checkout action to v3
• 🔐 Create all rate-limited PRs at once 🔐

---

⚠ Dependency Lookup Warnings ⚠

• Renovate failed to look up the following dependencies: Failed to look up maven package javax.annotation:javax.annotation-api.

Files affected: pom.xml

---

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

• fix(deps): update dependency com.fasterxml.jackson.core:jackson-databind to v2.13.4.2 [security]
• chore(deps): update dependency org.apache.maven.plugins:maven-assembly-plugin to v3.5.0

Detected dependencies

github-actions

.github/workflows/ci.yaml

• actions/checkout v2
• actions/cache v3
• gcr.io/gapic-images/googleapis-bazel 20210105

maven

pom.xml

• com.google.auto.value:auto-value 1.9
• com.google.auto.value:auto-value-annotations 1.9
• com.fasterxml.jackson.core:jackson-annotations 2.13.2
• com.fasterxml.jackson.core:jackson-core 2.13.2
• com.fasterxml.jackson.core:jackson-databind 2.13.2.1
• com.google.guava:guava 31.1-jre
• com.google.code.gson:gson 2.9.0
• javax.annotation:javax.annotation-api 1.3.2
• com.google.code.findbugs:jsr305 3.0.2
• junit:junit 4.13.2
• org.apache.maven.plugins:maven-assembly-plugin 3.3.0

Annotate Request Body messages as Required

When a request message appears in a discovery doc, for example:

message InsertForwardingRuleRequest {
  "insert": {
      //...
      "request": {
          "@ref": "ForwardingRule"
      }
      //... 
  }
  //...
}

This generates a request message with that resource on it:

message InsertForwardingRuleRequest {
  ForwardingRule forwarding_rule_resource = 32776239;
  //...
}

However, the required annotation is missing in the proto. As a result, the call in GAPIC PHP looks like the following:

$optionalParams = ['forwardingRule' => $forwardingRule];
$forwardingRulesClient->insert('project', 'region', $optionalParams);

When it would be better as this:

$optionalParams = [];
$forwardingRulesClient->insert('project', 'region', $forwardingRules, $optionalParams);

We should verify with the API teams that any message in a request body is required.

Add description comment for Request Body message

Related to #18
The message identified as a request body is getting added to a "request" message, but the proto field does not have a comment. The proto comment is used when generating documentation, and in some cases (such as PHP), it is required.

The request message missing a comment for the request body message:

message InsertForwardingRuleRequest {
  ForwardingRule forwarding_rule_resource = 32776239;
  //...
}

The message itself:

// Represents a Forwarding Rule resource.
//
// Forwarding rule resources in GCP can be either regional or global in scope:
//
// * [Global](/compute/docs/reference/rest/{$api_version}/globalForwardingRules) * [Regional](/compute/docs/reference/rest/{$api_version}/forwardingRules)
//
// A forwarding rule and its corresponding IP address represent the frontend configuration of a Google Cloud Platform load balancer. Forwarding rules can also reference target instances and Cloud VPN Classic gateways (targetVpnGateway).
//
// For more information, read Forwarding rule concepts and Using protocol forwarding.
//
// (== resource_for {$api_version}.forwardingRules ==) (== resource_for {$api_version}.globalForwardingRules ==) (== resource_for {$api_version}.regionForwardingRules ==)
message ForwardingRule {
  //...
}

We could potentially use the first sentence of the message description to describe the field in the request message, or something else. But it would be good (for docs generation purposes) to have some description for these fields!