genopenapi: Stable openapi names for nested types

[jgiles]

🚀 Feature

Summary

An option for output similar to the current openapi_naming_strategy=simple output but unconditionally qualifying nested type names rather than only doing so dynamically upon collision. This could be a modification of the behavior of the "simple" output (if the current behavior is viewed as a bug), or it could be a new option.

Current Behavior

Currently, strategies other than fqn dynamically determine how to qualify type names based on detecting collisions. This means that the generated names for a given type can change as new types are added to the genopapi invocation scope.

Current Behavior Example

Before File

Simple service getting entities Foo. Foo has a State.

syntax = "proto3";
package example.package;
option go_package = "github.com/example/packagepb";
import "google/api/annotations.proto";

service FooService {
    rpc GetFoo (GetFooRequest) returns (Foo) {
        option (google.api.http) = {
            get: "/foo"
        };
    }
}

message GetFooRequest {}

message Foo {
    enum State {
        STATUS_UNKNOWN = 0;
        PENDING = 1;
        FOOED = 2;
    }
    State state = 1;
}

After File

We add an entity Bar that also has a State in the same proto package.

syntax = "proto3";
package example.package;
option go_package = "github.com/example/packagepb";
import "google/api/annotations.proto";

service FooService {
    rpc GetFoo (GetFooRequest) returns (Foo) {
        option (google.api.http) = {
            get: "/foo"
        };
    }
}

message GetFooRequest {}

message Foo {
    enum State {
        STATUS_UNKNOWN = 0;
        PENDING = 1;
        FOOED = 2;
    }
    State state = 1;
}

service BarService {
    rpc GetBar (GetBarRequest) returns (Bar) {
        option (google.api.http) = {
            get: "/bar"
        };
    }
}

message GetBarRequest {}

message Bar {
    enum State {
        STATUS_UNKNOWN = 0;
        PENDING = 1;
        BARRED = 2;
    }
    State state = 1;
}

Emitted Type Names With Each openapi_naming_strategy

• legacy
  • Before: FooState
  • After: packageFooState, packageBarState
  • Notes: Adding another type changed the existing type's name, and the generated names in "after" are unnecessarily qualified with package
• fqn
  • Before: example.package.Foo.State
  • After: example.package.Foo.State, example.package.Bar.State
  • Notes: The output is predictable and stable but overly verbose for most use cases.
• simple
  • Before: State
  • After: Foo.State, Bar.State
  • Notes: Adding another type changed the existing type's name. In the "Before" name, the generic "State" is not very meaningful - the name is meant to be meaningful in the context of Foo. The After output is desirable for that case (clear, no collisions, not overly verbose).

Desired Behavior

Always output Foo.State for the nested type (unless Foo.State collides with a name from another package, in which case qualify it like package.Foo.State). This is like in the "after" output from the simple strategy, but it would not depend on whether another type Bar.State happened to be declared in the package.

In general, it would be nice to have options for more predictable and explicit control of name behavior in OpenAPI generation, rather than trying to "fix" collisions automatically as they are discovered at generation time. That is a larger issue, because there are complexities when colliding types are pulled in from other packages (I use State in this example because Status collides with the common protobuf Status type, which complicates the example). However, within a given package we should be able to mirror how nested-type names are generated in other language plugins (by qualifying the nested types), and as long as we use a separator that cannot appear in the proto type names themselves we will not have collisions within the package (see #1066 for an issue where qualification without separators can still result in collisions).

[johanbrandhorst]

Hi Josh, thanks for the feature request! This is a nice suggestion, and I don't think it would hurt to have more options here. I don't think we should change what simple does, so maybe something like package to make it relative to the package definition (sounds like what you want)? What do you think?

[jgiles]

Hey @johanbrandhorst , I've put up a simple PR adding a "package" naming strategy implementing the requested behavior: #4372

Please let me know if that looks like the right approach!