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

Generating openapi from proto gives field of type string and format RFC3339 but not vice versa #310

Closed
panperla opened this issue Feb 15, 2022 · 2 comments

Comments

@panperla
Copy link

I need such output in my proto

...
message XYZ {
    string versionName = 1 [json_name = "versionname"];
    string branchName = 2 [json_name = "branchname"];
    string appName = 3 [json_name = "appname"];
    string appDomainName = 4 [json_name = "appdomainname"];
    string appURL = 5 [json_name = "appurl"];
    string commitSHA = 6 [json_name = "commitsha"];
    google.protobuf.Timestamp responseTime = 7 [json_name = "responsetime"];
}
...

During the conversion from openapi google.protobuf.Timestamp type is being just translated as string.
In openapi I'm defining it as a:

responsetime:
                    type: string
                    format: RFC3339

To conclude generating openapi file from proto gives me field with type string and format RFC3339 but when I convert it back to proto the google.protobufTImestamp is lost.

I might be missing something please point me into right direction how I can arrive with google.protobuf.Timestamp responseTime from openapi.

@adolfo
Copy link
Contributor

adolfo commented Feb 22, 2022

I'm having a similar issue, but under a slightly different use case. I'm going from Protobuf to OpenAPI to a Rest Client. In that process I'm losing the timestamp type information and ending up with a plain string as well.

I believe this is caused by an invalid string format in the generated OpenAPI spec. Specifically, the timestamp format "RFC3339" isn't a valid format per the OpenAPI docs. It states that the "date-time" format already conforms to RFC 3339:

OpenAPI defines the following built-in string formats: [...] date-time – the date-time notation as defined by RFC 3339, section 5.6, for example, 2017-07-21T17:32:28Z source1 source2

It appears gnostic is inserting "RFC3339" where "date-time" appears to be the correct format value:

case ".google.protobuf.Timestamp":
// Timestamps are serialized as strings
return &v3.SchemaOrReference{
Oneof: &v3.SchemaOrReference_Schema{
Schema: &v3.Schema{Type: "string", Format: "RFC3339"}}}

I propose that the "RFC3339" format be replaced with the correct "date-time" format. This will enable other tools such as OpenAPI-Generator, etc. to do the right thing when it comes to generating timestamps for each respective language.

It's not clear if this was an intentional value that is used downstream, or a simple oversight, but I'd gladly open a PR if the maintainers agree with this change. /cc @morphar @timburks

@adolfo
Copy link
Contributor

adolfo commented Mar 16, 2022

Addressed in #319

jeffsawatzky added a commit to jeffsawatzky/gnostic that referenced this issue Apr 4, 2022
timburks pushed a commit that referenced this issue Apr 5, 2022
* fix the timestamp format. (#310)
* update openapi_default_response.yaml to expect "date-time" instead of "RFC3339".
@timburks timburks closed this as completed Apr 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants