sudorandom / protoc-gen-connect-openapi

Plugin for generating OpenAPIv3 from protobufs matching the Connect RPC interface
MIT License
89 stars 5 forks source link

Add support for predefined constraints #35

Open dropwhile opened 2 weeks ago

dropwhile commented 2 weeks ago

Using the rather new (released within the last week or two as part of protovalidate-go v0.8.0 from this pr) support for predefined constraints, the openapi output looks a little odd.

In and effort to reduce boilerplate, I went from a definition of this:

message EarmarkCreateRequest {
     string event_item_ref_id = 1 [(buf.validate.field).cel = {
       id: "refid.format"
       message: "must be in refid format"
       expression: "this.matches('^[0-9a-zA-Z]{26}$')"
     }];
}

To this:

edition = "2023";

extend buf.validate.StringRules {
  bool refid = 70000001 [(buf.validate.predefined).cel = {
    id: "string.refid"
    expression: "this.matches('^[0-9a-zA-Z]{26}$')"
    message: "must be in refid format"
  }];
}

message EarmarkCreateRequest {
    // (buf.validate.field).string.(refid) is now re-usable across many other messages
    // that all have the same field type
     string event_item_ref_id = 1 [(buf.validate.field).string.(refid) = true];
}

And the openapi output changed thusly:

         eventItemRefId:
           type: string
           title: event_item_ref_id
-          description: |+
-            must be in refid format:
-            ```
-            this.matches('^[0-9a-zA-Z]{26}$')
-            ```
-
+          description: |-
+            string event_item_ref_id = 1 [(buf.validate.field).cel = {
+            id: "refid.format"
+            message: "must be in refid format"
+            expression: "this.matches('^[0-9a-zA-Z]{26}$')"
+            }];

Is this description change expected? If using a predefined constraint, it would be nice to retain the simpler and more easily readable description, if possible (eg. drill down to show the message and expression like before).

sudorandom commented 2 weeks ago

Yeah, this tool doesn't yet support predefined constraints... but I'm curious why it's outputting that way for you because I'm not getting what you're showing at all. This is what I get:

            +        eventItemRefId:
            +          type: string
            +          title: event_item_ref_id
            +          description: |-
                +            (buf.validate.field).string.(refid) is now re-usable across many other messages
                +             that all have the same field type

It looks like I need to add some code to specifically get the predefined constraints add add it to the description like the unpredefined version. I'm going to adjust your title a little bit to reflect this.

dropwhile commented 1 week ago

but I'm curious why it's outputting that way for you because I'm not getting what you're showing at all

My output was different because the comment in my example was just added to show some reasoning why I was doing it for the GH issue. The actual code lacked that comment block.

sudorandom commented 1 week ago

Oh got it! Understood! But yeah, thanks for the issue! This should be quick but I am asking buf folks about the best way to get this information since they don't yet provide an easy interface for getting constraints defined this way.

dropwhile commented 1 week ago

Sounds good @sudorandom! 👍

sudorandom commented 1 week ago

Related: https://github.com/bufbuild/protovalidate-go/issues/147