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

Add support for predefined constraints #35

Open
dropwhile opened this issue Sep 28, 2024 · 6 comments
Open

Add support for predefined constraints #35

dropwhile opened this issue Sep 28, 2024 · 6 comments

Comments

@dropwhile
Copy link

dropwhile commented Sep 28, 2024

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
Copy link
Owner

sudorandom commented Sep 29, 2024

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.

@sudorandom sudorandom changed the title Formatting of predefined constraints seems odd. Add support for predefined constraints Sep 29, 2024
@dropwhile
Copy link
Author

dropwhile commented Sep 29, 2024

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
Copy link
Owner

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
Copy link
Author

Sounds good @sudorandom! 👍

@sudorandom
Copy link
Owner

Related: bufbuild/protovalidate-go#147

@dropwhile
Copy link
Author

dropwhile commented Nov 20, 2024

@sudorandom looks like the upstream change (addition of ResolvePredefinedConstraints) made it into a protovalidate-go release.
https://github.com/bufbuild/protovalidate-go/releases/tag/v0.7.3

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

2 participants