dimas-b commented on code in PR #4621:
URL: https://github.com/apache/polaris/pull/4621#discussion_r3364704414
##########
server-templates/pojo.mustache:
##########
@@ -90,7 +99,23 @@ It is updated to remove all swagger annotations and support
builders and immutab
return new Builder();
}
{{#hasRequired}}
- public static Builder builder({{#requiredVars}}{{{datatypeWithEnum}}}
{{name}}{{^-last}}, {{/-last}}{{/requiredVars}}) {
+ public static Builder
builder({{#requiredVars}}{{#isContainer}}{{#isMap}}{{{baseType}}}<String,
{{{items.baseType}}}>{{/isMap}}{{^isMap}}{{{baseType}}}<{{{items.baseType}}}>{{/isMap}}{{/isContainer}}{{^isContainer}}{{{datatypeWithEnum}}}{{/isContainer}}
{{name}}{{^-last}}, {{/-last}}{{/requiredVars}}) {
Review Comment:
The "validated" method does not appear to be called in runtime... TBH, I'm
not sure it is worth "fixing" it. I'd prefer removing it in favour of the
no-arg method.
Builders traditionally start empty, accept parameter via various "set"
methods, then validate data as a whole at `.build()` time. I'm not sure what
value the parameterized `.builder(...)` method brings.
That said, even the regular `PrincipalRole` constructor does not appear to
trigger validation. I did a quick test and was able to create a principal role
with the name `system` via the Management REST API, which is supposed to be
blocked by validation annotations.
I believe the first step in addressing #3716 is probably to double checks
how Hibernate validation is engaged in Polaris... and whether it is engaged at
all 🤔
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
