You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by GitBox <gi...@apache.org> on 2021/10/18 13:01:34 UTC
[GitHub] [thrift] BytedanceRPC opened a new pull request #2469: THRIFT-5423: Support go parameter validation in IDL
BytedanceRPC opened a new pull request #2469:
URL: https://github.com/apache/thrift/pull/2469
<!-- Explain the changes in the pull request below: -->
Feature Metrix:
`vt.const`: must be specified value
`vt.lt`: less than the specified value
`vt.le`: less than or equal specified value
`vt.gt`: great than the specified value
`vt.ge`: great than or equal specified value
`vt.in`: must be in specified values
`vt.not_in`: must not be in specified values
`vt.min_size`: min size
`vt.max_size`: max size
`vt.pattern`: regexp pattern
`vt.prefix`: prefix must be specified value
`vt.suffix`: suffix must be specified value
`vt.contains`: must contain specified value
`vt.not_contains`: must not contain specified value
`vt.defiend_only`: must be defined value
`vt.no_sparse`: map value must be non-nil pointer
I notice that Thrift IDL has no user-defined parameter validating mechanism
<!-- We recommend you review the checklist/tips before submitting a pull request. -->
- [x] Did you create an [Apache Jira](https://issues.apache.org/jira/browse/THRIFT-5423) ticket? (not required for trivial changes)
- [x] If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
- [x] Did you squash your changes to a single commit? (not required, but preferred)
- [x] Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
- [ ] If your change does not involve any code, include `[skip ci]` anywhere in the commit message to free up build resources.
<!--
The Contributing Guide at:
https://github.com/apache/thrift/blob/master/CONTRIBUTING.md
has more details and tips for committing properly.
-->
--
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: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] BytedanceRPC commented on a change in pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
BytedanceRPC commented on a change in pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#discussion_r809031620
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,180 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64 | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64 | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "[1, 2, 4]")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
Review comment:
Considering that it will be implemented in many languages, it is more appropriate to use BRE (basic regular expression) here. I will add it to the table.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G edited a comment on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
Jens-G edited a comment on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-949004501
PS: Is that IsValid() called on reads and writes or only at one of these? Or do I have to invoke it explicitly by calling IsValid()?
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] fishy commented on a change in pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
fishy commented on a change in pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#discussion_r808568115
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,180 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64 | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64 | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
Review comment:
in the table above they are defined as `le`/`gt`, not `lte`/`gte`.
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,180 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64 | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64 | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "[1, 2, 4]")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
+ 4: string DebugInfo (vt.prefix = "[Debug]")
+ 5: string PanicInfo (vt.contains = "panic")
+}
+```
+- Bool
+```
+struct BoolDemo {
+ 1: bool AMD (vt.const = "true")
+}
+```
+- Enum
+```
+struct EnumDemo {
+ 1: Type AddressType (vt.const = "String")
+ 2: Type ValueType (vt.defined_only = "true")
+}
+```
+- Set/List
+```
+struct SetListDemo {
+ 1: list<string> Persons (vt.min_size = "5", vt.max_size = "10")
+ 2: set<double> HealthPoints (vt.elem.gt = "0")
+}
+```
+- Map
+```
+struct MapDemo {
+ 1: map<i32, string> IdName (vt.min_size = "5", vt.max_size = "10")
+ 2: map<i32, DemoTestRequest> Requests (vt.no_sparse = "true")
Review comment:
what does `vt.no_sparse` mean? I don't see it defined anywhere in the proposal.
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,180 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64 | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64 | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "[1, 2, 4]")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
+ 4: string DebugInfo (vt.prefix = "[Debug]")
+ 5: string PanicInfo (vt.contains = "panic")
+}
+```
+- Bool
+```
+struct BoolDemo {
+ 1: bool AMD (vt.const = "true")
+}
+```
+- Enum
+```
+struct EnumDemo {
+ 1: Type AddressType (vt.const = "String")
+ 2: Type ValueType (vt.defined_only = "true")
+}
+```
+- Set/List
+```
+struct SetListDemo {
+ 1: list<string> Persons (vt.min_size = "5", vt.max_size = "10")
+ 2: set<double> HealthPoints (vt.elem.gt = "0")
+}
+```
+- Map
+```
+struct MapDemo {
+ 1: map<i32, string> IdName (vt.min_size = "5", vt.max_size = "10")
+ 2: map<i32, DemoTestRequest> Requests (vt.no_sparse = "true")
+ 3: map<i32, double> Some, (vt.key.gt = "0", vt.value.lt = "1000")
Review comment:
this seems to be the only line in all the examples that has the trailing comma. without validator rules thrift is very lenient on trailing comma, so that's fine, but when we actually have trailing comma, it likely makes more sense to have it after the validator rules? so this line should be this instead:
```suggestion
3: map<i32, double> Some (vt.key.gt = "0", vt.value.lt = "1000"),
```
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,180 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64 | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64 | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "[1, 2, 4]")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
+ 4: string DebugInfo (vt.prefix = "[Debug]")
+ 5: string PanicInfo (vt.contains = "panic")
+}
+```
+- Bool
+```
+struct BoolDemo {
+ 1: bool AMD (vt.const = "true")
+}
+```
+- Enum
+```
+struct EnumDemo {
+ 1: Type AddressType (vt.const = "String")
Review comment:
I guess here in this example `String` is one of the values of enum `Type`? it helps to add that into the example (add the definition of `Type` here).
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,180 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64 | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64 | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "[1, 2, 4]")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
Review comment:
I don't see `pattern` defined in the table. for a regexp there are so many flavors, I'd strongly suggest to either define the flavor here, or just remove it altogether.
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,180 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64 | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64 | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "[1, 2, 4]")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
+ 4: string DebugInfo (vt.prefix = "[Debug]")
+ 5: string PanicInfo (vt.contains = "panic")
+}
+```
+- Bool
+```
+struct BoolDemo {
+ 1: bool AMD (vt.const = "true")
+}
+```
+- Enum
+```
+struct EnumDemo {
+ 1: Type AddressType (vt.const = "String")
Review comment:
also according to the table `const` can only be used on string and bool types, while an enum is technically an int type. I also don't see why `in` and `not_in` can only be used for int types but not string/bool types. all in all I still don't see anything `const` can do that `in` cannot so I'd suggest just use `in` and remove `const`.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] fishy commented on a change in pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
fishy commented on a change in pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#discussion_r780519590
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,182 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "1", validator.in = "2", validator.in = "4")
+ 3: i64 MagicNumber (validator.const = "0x5f3759df")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
+ 4: string DebugInfo (vt.prefix = "[Debug]")
+ 5: string PanicInfo (vt.contains = "panic")
+ 6: string Editor (vt.in = "vscode", vt.in = "vim", vt.in = "goland")
Review comment:
This is too subtle to be in a spec. At the very least, it should be explicitly defined/discussed in the spec. But I would rather we have a spec that has clear definitions of and/or logical relations in the rules (e.g. use `and`/`or` or `&&`/`||` explicitly over `,`)
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] fishy commented on a change in pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
fishy commented on a change in pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#discussion_r780519590
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,182 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "1", validator.in = "2", validator.in = "4")
+ 3: i64 MagicNumber (validator.const = "0x5f3759df")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
+ 4: string DebugInfo (vt.prefix = "[Debug]")
+ 5: string PanicInfo (vt.contains = "panic")
+ 6: string Editor (vt.in = "vscode", vt.in = "vim", vt.in = "goland")
Review comment:
This is too subtle to be in a spec. At the very least, it should be explicitly defined/discussed in the spec. But I would rather we have a spec that has clear definitions of and/or logical relations in the rules (e.g. use `and`/`or` or `&&`/`||` explicitly over `,`)
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] BytedanceRPC commented on a change in pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
BytedanceRPC commented on a change in pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#discussion_r812892752
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,187 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
Review comment:
It actually uses peg grammar, but with the wrong symbol, I made a fix
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] BytedanceRPC commented on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
BytedanceRPC commented on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-973808600
@Jens-G Could you help review the code or assign it to someone else for review?
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] BytedanceRPC commented on a change in pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
BytedanceRPC commented on a change in pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#discussion_r812894952
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,187 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64, enum | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64, enum | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contains | must contain | string | string | - |
+| not_contains | must not contain | string | string | - |
+| pattern | basic regular expression | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
Review comment:
>
I add an explicit definition for commas between rules and also a reference for basic regular expression.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G commented on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
Jens-G commented on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-945841531
What does "vt" mean? Virtual Table? Verschwörungstheorie? VhaT else?
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G edited a comment on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
Jens-G edited a comment on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-945841531
What does "vt" mean? Virtual Table? Verschwörungstheorie? VhaT else?
> defiend_only
The good news is In the code it is spelled correctly. :-D
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] BytedanceRPC commented on a change in pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
BytedanceRPC commented on a change in pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#discussion_r809059967
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,180 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64 | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64 | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "[1, 2, 4]")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
+ 4: string DebugInfo (vt.prefix = "[Debug]")
+ 5: string PanicInfo (vt.contains = "panic")
+}
+```
+- Bool
+```
+struct BoolDemo {
+ 1: bool AMD (vt.const = "true")
+}
+```
+- Enum
+```
+struct EnumDemo {
+ 1: Type AddressType (vt.const = "String")
Review comment:
`vt.in` and `vt.not` will be implemented in the form of an array, and the value of annotation is a string, which means that we need to escape special characters. In `vt.in`, we use `,` as separators which means we must use `"` or `'` to identify the value of the string, which means that we have to escape again, which will make `vt.in` and `vt.not_in` are very complex. That's why in previous versions we defined different values by defining multiple `vt.in` instead of defining multiple values in one `vt.in`.
To sum up, we think it is inappropriate to use `vt.in` or `vt.not_in` in string type. If there is such a requirement, it can be implemented by `pattern`.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] BytedanceRPC commented on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
BytedanceRPC commented on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-958691924
> Re "sparse": It was not about the feature, but about terminology. A sparse array is an array where most values are unused (e.g. NULL/nil), and the most significant problem about it is to store it with a minimum amount of memory, e.g. imagine an 2-sided matrix with 5 mio by 5 mio possible combinations, where only a few thousand are really used. The challenge here is to get it small AND still fast. With that in mind, a map or dictionary is by definition an efficient, sparse data structure with something in the range of O(1) for reads. Hence, "sparse map" makes not much sense, plus what you really mean is sth completely different: The entry is used, only the value happens to be null. An existing null entry and a non-existing entry are two different things.
Thanks for explanation.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] fishy commented on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
fishy commented on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-990371184
> vt.in: must be in specified values
> vt.not_in: must not be in specified values
it's not clear to me whether that is an enumeration (e.g. `vt.in = "[1, 2, 4, 8]"` or a range (e.g. `vt.in = "[1, 4]"`), and if it's a range, are ends inclusive or exclusive?
I think it would help if you can include the markdown file you attached to the JIRA ticket into this PR so I can review the spec first (and it will be easier to give inline feedback to the spec there), before I review the implementation code.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G edited a comment on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
Jens-G edited a comment on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-945841531
> Prefix "vt" stands for validation
That would have not been my first guess. Mmmh. Ok.
> defiend_only
The good news is In the code it is spelled correctly :-)
> no_sparse map value must be non-nil pointer
Not sure if I can agree. "Sparse" in the context of memory management means usually something else.
Bonus question: I only skimmed the code so I probably overlooked it, but when and where is it called?
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] BytedanceRPC commented on a change in pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
BytedanceRPC commented on a change in pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#discussion_r793548292
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,182 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "1", validator.in = "2", validator.in = "4")
+ 3: i64 MagicNumber (validator.const = "0x5f3759df")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
+ 4: string DebugInfo (vt.prefix = "[Debug]")
+ 5: string PanicInfo (vt.contains = "panic")
+ 6: string Editor (vt.in = "vscode", vt.in = "vim", vt.in = "goland")
Review comment:
Great suggestion, I updated the proposal and modified the code according to the proposal, please review it again
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] BytedanceRPC commented on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
BytedanceRPC commented on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-994531708
> > vt.in: must be in specified values
> > vt.not_in: must not be in specified values
>
> it's not clear to me whether that is an enumeration (e.g. `vt.in = "[1, 2, 4, 8]"` or a range (e.g. `vt.in = "[1, 4]"`), and if it's a range, are ends inclusive or exclusive?
>
> I think it would help if you can include the markdown file you attached to the JIRA ticket into this PR so I can review the spec first (and it will be easier to give inline feedback to the spec there), before I review the implementation code.
Sorry for not getting back to you sooner. Value of 'vt.in' must be specified，it works like below
```thrift
3: i8 Byte0 = 1 (vt.in = "0", vt.in = "1", vt.in = "2", vt.not_in = "3", vt.not_in = "4", vt.not_in = "5")
```
We update thrift-parameter-validation-proposal, and put it in [doc/proposal/thrift-parameter-validation-proposal.md](https://github.com/apache/thrift/pull/2469/files?file-filters%5B%5D=.md). Thanks for your advice, and welcome to comment.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] fishy commented on a change in pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
fishy commented on a change in pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#discussion_r771719971
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,182 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "1", validator.in = "2", validator.in = "4")
+ 3: i64 MagicNumber (validator.const = "0x5f3759df")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
+ 4: string DebugInfo (vt.prefix = "[Debug]")
+ 5: string PanicInfo (vt.contains = "panic")
+ 6: string Editor (vt.in = "vscode", vt.in = "vim", vt.in = "goland")
Review comment:
in the example on field 2 (`vt.min_size = "6", vt.max_size = "12"`), `,` between rules means **and**, otherwise min 6 or max 12 just mean everything and doesn't make any sense.
but in the example on field 6 (`vt.in = "vscode", vt.in = "vim", vt.in = "goland"`), `,` between rules means **or**, if it means "and" then this rule won't match anything.
so as a first thing, we need a consistent way to define the logical relation between multiple rules.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] BytedanceRPC commented on a change in pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
BytedanceRPC commented on a change in pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#discussion_r809059967
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,180 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64 | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64 | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "[1, 2, 4]")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
+ 4: string DebugInfo (vt.prefix = "[Debug]")
+ 5: string PanicInfo (vt.contains = "panic")
+}
+```
+- Bool
+```
+struct BoolDemo {
+ 1: bool AMD (vt.const = "true")
+}
+```
+- Enum
+```
+struct EnumDemo {
+ 1: Type AddressType (vt.const = "String")
Review comment:
`vt.int` and `vt.not` will be implemented in the form of an array, and the value of annotation is a string, which means that we need to escape special characters. In `vt.in`, we use `,` as separators which means we must use `"` or `'` to identify the value of the string, which means that we have to escape again, which will make `vt.in` and `vt.not_in` are very complex. That's why in previous versions we defined different values by defining multiple `vt.in` instead of defining multiple values in one `vt.in`.
To sum up, we think it is inappropriate to use `vt.in` or `vt.not_in` in string type. If there is such a requirement, it can be implemented by `pattern`.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] BytedanceRPC commented on a change in pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
BytedanceRPC commented on a change in pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#discussion_r809032540
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,180 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64 | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64 | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "[1, 2, 4]")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
+ 4: string DebugInfo (vt.prefix = "[Debug]")
+ 5: string PanicInfo (vt.contains = "panic")
+}
+```
+- Bool
+```
+struct BoolDemo {
+ 1: bool AMD (vt.const = "true")
+}
+```
+- Enum
+```
+struct EnumDemo {
+ 1: Type AddressType (vt.const = "String")
+ 2: Type ValueType (vt.defined_only = "true")
+}
+```
+- Set/List
+```
+struct SetListDemo {
+ 1: list<string> Persons (vt.min_size = "5", vt.max_size = "10")
+ 2: set<double> HealthPoints (vt.elem.gt = "0")
+}
+```
+- Map
+```
+struct MapDemo {
+ 1: map<i32, string> IdName (vt.min_size = "5", vt.max_size = "10")
+ 2: map<i32, DemoTestRequest> Requests (vt.no_sparse = "true")
Review comment:
Sorry, it's a deprecated key, I forgot to update the example.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G commented on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
Jens-G commented on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-949004501
PS: How do I use it?
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] BytedanceRPC commented on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
BytedanceRPC commented on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-958694210
> PS: Is that IsValid() called on reads and writes or only at one of these? Or do I have to invoke it explicitly by calling IsValid()?
For now, go generator only provides 'generate_validator' flag to generate IsValid() method, so we have to call IsValid() to validate the structure date.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G edited a comment on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
Jens-G edited a comment on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-945841531
> Prefix "vt" stands for validation
That would have not been my first guess. Mmmh. Ok.
> defiend_only
The good news is In the code it is spelled correctly :-)
> no_sparse map value must be non-nil pointer
Not sure if I can agree. "Sparse" in the context of memory management means usually something else.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] BytedanceRPC commented on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
BytedanceRPC commented on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-946430113
Thanks for the quick reply.
> > Prefix "vt" stands for validation
>
> That would have not been my first guess. Mmmh. Ok.
Yes, 'vt' stands for validation, considering that 'vt' might make fewer conflicts.
> > defiend_only
>
> The good news is In the code it is spelt correctly :-)
I have fixed the comment.
> > no_sparse map value must be non-nil pointer
>
> Not sure if I can agree. "Sparse" in the context of memory management means usually something else.
>
> Bonus question: I only skimmed the code so I probably overlooked it, but when and where is it called?
`no_sparse` is designed for go, since like `map<string, StructFoo>` is a pretty common data structure in web service, and a nil value could lead to an unexpected result (an empty struct) after deserilization. But not sure it's a common problem for all languages, so `no_sparse` is not implemented. I will remove it from the feature matrix.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] BytedanceRPC commented on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
BytedanceRPC commented on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-958691924
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] simon0-o commented on a change in pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
simon0-o commented on a change in pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#discussion_r778593343
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,182 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "1", validator.in = "2", validator.in = "4")
+ 3: i64 MagicNumber (validator.const = "0x5f3759df")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
+ 4: string DebugInfo (vt.prefix = "[Debug]")
+ 5: string PanicInfo (vt.contains = "panic")
+ 6: string Editor (vt.in = "vscode", vt.in = "vim", vt.in = "goland")
+}
+```
+- Bool
+```
+struct BoolDemo {
+ 1: bool AMD (vt.const = "true")
+}
+```
+- Enum
+```
+struct EnumDemo {
+ 1: Type AddressType (vt.const = "String")
Review comment:
"vt.const" and "vt.in" both are used to limit the range of values. The difference is that "vt.const" only specifies one value, "vt.in" multiple values can be specified
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] simon0-o commented on a change in pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
simon0-o commented on a change in pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#discussion_r778596067
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,182 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "1", validator.in = "2", validator.in = "4")
+ 3: i64 MagicNumber (validator.const = "0x5f3759df")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
+ 4: string DebugInfo (vt.prefix = "[Debug]")
+ 5: string PanicInfo (vt.contains = "panic")
+ 6: string Editor (vt.in = "vscode", vt.in = "vim", vt.in = "goland")
Review comment:
The logical relation of all rules is "and", " vt.in " and "vt.not _in" are two very special rules, all " vt.in " will be merged into one rule, so is "vt.not _in"
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] BytedanceRPC commented on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
BytedanceRPC commented on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-1048788066
>
I add an explicit definition for commas between rules and also a reference for basic regular expression.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] fishy commented on a change in pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
fishy commented on a change in pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#discussion_r771721791
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,182 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contain | must contain | string | string | - |
+| not_contain | must not contain | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
+
+#### 3.2.2 IDL example
+- Number
+```
+struct NumericDemo{
+ 1: double Value (validator.gte = "1000.1", validator.lte = "10000.1")
+ 2: i8 Type (validator.in = "1", validator.in = "2", validator.in = "4")
+ 3: i64 MagicNumber (validator.const = "0x5f3759df")
+}
+```
+- String/Binary
+```
+struct StringDemo{
+ 1: string Uninitialized (vt.const = "abc")
+ 2: string Name (vt.min_size = "6", vt.max_size = "12")
+ 3: string SomeStuffs (vt.pattern = "[0-9A-Za-z]+")
+ 4: string DebugInfo (vt.prefix = "[Debug]")
+ 5: string PanicInfo (vt.contains = "panic")
+ 6: string Editor (vt.in = "vscode", vt.in = "vim", vt.in = "goland")
+}
+```
+- Bool
+```
+struct BoolDemo {
+ 1: bool AMD (vt.const = "true")
+}
+```
+- Enum
+```
+struct EnumDemo {
+ 1: Type AddressType (vt.const = "String")
Review comment:
with the above example of `vt.in`, I don't see how `vt.const` is different from `vt.in`? can you explain the differences between them?
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G commented on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
Jens-G commented on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-949003828
Re "sparse": It was not about the feature, but about terminology. A sparse array is an array where most values are unused (e.g. NULL/nil), and the most significant problem about it is to store it with a minimum amount of memory, e.g. imagine an 2-sided matrix with 5 mio by 5 mio possible combinations, where only a few thousand are really used. The challenge here is to get it small AND still fast. With that in mind, a map or dictionary is by definition an efficient, sparse data structure with something in the range of O(1) for reads. Hence, "sparse map" makes not much sense, plus what you really mean is sth completely different: The entry is used, only the value happens to be null. An existing null entry and a non-existing entry are two different things.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] BytedanceRPC commented on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
BytedanceRPC commented on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-958691924
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] fishy commented on a change in pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
fishy commented on a change in pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#discussion_r812311809
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,187 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
Review comment:
disclaimer: I'm not an expert in BNF nor ABNF. but looking at wikipedia, in ABNF an optional sequence is using `[]` instead of `()?`: https://en.wikipedia.org/wiki/Augmented_Backus%E2%80%93Naur_form#Optional_sequence
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,187 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
Review comment:
and for this it should be `0*()`: https://en.wikipedia.org/wiki/Augmented_Backus%E2%80%93Naur_form#Variable_repetition
##########
File path: doc/proposal/thrift-parameter-validation-proposal.md
##########
@@ -0,0 +1,187 @@
+# Thrift Parameter Validation Proposal
+
+> Version 1.1
+>
+> Dec 15, 2021
+>
+> duanyi.aster@bytedance.com, wangtieju@bytedance.com
+
+### 1. Abstract
+***
+This document presents a proposed set of annotations to the Thrift IDL. The new annotations will supports parameter validation using build-in or third-party validators. The goal of this proposal is to define sematics and behavior of validation annotations, rather than to discuss their implementation.
+
+### 2. Background
+***
+Parameter validation is a common need for web service. In the past, we usually write our validating logics after a RPC message deserialized by thrift. This ways works flexibly enough but restrict poorly: It is dangerous than service A and service B using the same IDL have two different validating rule, which often misdirects developers. Even if we extract our validating codes to a single module, simple and repeated work (ex. `if xx.Field1 > 1 then ...`) is really disturbing. If we can use build tool to generating codes for simple and unchangeable restraint, the web service will be more robust and developers will benefits from lighter work.
+Compared to other IDL, the parameter validation gradually gets strong commutity supports like PGV ([protoc-gen-validate](https://github.com/envoyproxy/protoc-gen-validate)), benefiting from pb's strong plugin mechanism (lacking official plugin mechanism is one reason for we submit this proposal). Take a long-term view, auto-generated parameter validation may be a step towards code-less web service.
+
+### 3. Proposal
+***
+This proposal includes three part: Validate Annotation Sematics, Validate Rule and Validate Feedback. The first declare how to write a validate annotation, the middle explain how every annotation should behave, the last introduces a mechanism of validating feedback.
+
+#### 3.1 Validate Annotation Sematics
+This sematics uses same rule of [Thrift IDL](https://thrift.apache.org/docs/idl). The validate option only works on struct fields, thus we must start from Field sematics part.
+- Field
+```apache
+Field ::= FieldID? FieldReq? FieldType Identifier ('=' ConstValue)? ValidateAnnotations? ListSeparator?
+```
+- ValidateAnnotations
+```apache
+ValidateAnnotations ::= '(' ValidateRule+ ListSeparator? ')'
+```
+- ValidateRule
+```apache
+ValidateRule := ('validate' | 'vt') Validator+ = '"' ValidatingValue? '"'
+```
+- Validator
+
+ Build-in validating logics. See [Supported Validator](#321-supported-validator) part.
+```apache
+Validator = '.' Identifier
+```
+- ValidatingValue
+```apache
+ValidatingValue := (ToolFunction '(' )? Arguments ‘)’?
+```
+- ToolFunction
+
+ Build-in or user-defined tool functions. See [Tool Function](#325-tool-function) part.
+```apache
+ToolFunction := '@' Identifier
+```
+- Arguments
+```apache
+Arguments := (DynamicValue ListSeparator?)*
+```
+- DynamicValue
+```apache
+DynamicValue := ConstValue | FieldReference
+```
+- FieldReference
+
+ See [Field Reference](#324-field-reference) part.
+```apache
+FieldReference := '$' ReferPath
+ReferPath := FieldName?(('['IntConstant']')|('.'Identifier))?
+```
+- All other sematics keep same with [standard definition](https://thrift.apache.org/docs/idl)
+
+### 3.2 Validate Rule
+The validate rule is works as a Boolean Expression, and Validator is core logic for one validate rule. Every Validator works like an Operator, calculating the Validating Value and Field Value, and then compare. For example, `gt` (greater than) will compare the right Validating Value with value of the field it belongs to, and return `true` if field value is greater than value or `false` if field value is not. We appoint that: Only if the validate rule returns true, the validated parameter is valid.
+
+#### 3.2.1 Supported Validator
+Below lists the support validators. Value type means the type of validating value, field type means type of validated field.
+
+| validator | behavior | value type | field type | secodary validator |
+| ------------ | -------------------------------- | ------------------------------------ | ---------------------- | ------------------ |
+| const | must be constant | string, bool | same with value | - |
+| defined_only | must be defined value | enum | enum | - |
+| not_nil | must not be empty | "true" | any | - |
+| skip | skip validate | "true" | any | - |
+| eq | equals to (`==`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| ne | not equals to (`!=`) | i8, i16, i32, i64, f64, string, bool | same with value | - |
+| lt | less than (`<`) | i8, i16, i32, i64, f64 | same with value | - |
+| le | less equal (`<=`) | i8, i16, i32, i64, f64 | same with value | - |
+| gt | greater than (`>`) | i8, i16, i32, i64, f64 | same with value | - |
+| ge | greater equal (`>=`) | i8, i16, i32, i64, f64 | same with value | - |
+| in | within given container | i8, i16, i32, i64, f64, enum | same with value | - |
+| not_in | not within given container | i8, i16, i32, i64, f64, enum | same with value | - |
+| elem | field's element constraint | any | list, set | support |
+| key | field's element key constraint | any | map | support |
+| value | field's element value constraint | any | map | support |
+| min_size | minimal length | i8, i16, i32, i64 | string, list, set, map | - |
+| max_size | maximal length | i8, i16, i32, i64 | string, list, set, map | - |
+| prefix | field prefix must be | string | string | - |
+| suffix | suffix must be | string | string | - |
+| contains | must contain | string | string | - |
+| not_contains | must not contain | string | string | - |
+| pattern | basic regular expression | string | string | - |
+
+- Secodary validator is a successive validator, usually used at container-type field. See below Set/List/Map examples.
Review comment:
I would prefer to explicitly define `elem` used in examples here as well.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G edited a comment on pull request #2469: THRIFT-5423: Support go parameter validation in IDL
Posted by GitBox <gi...@apache.org>.
Jens-G edited a comment on pull request #2469:
URL: https://github.com/apache/thrift/pull/2469#issuecomment-945841531
What does "vt" mean? Virtual Table? Verschwörungstheorie? VhaT else?
> defiend_only
The good news is In the code it is spelled correctly :-)
> no_sparse map value must be non-nil pointer
Not sure if I can agree. "Sparse" in the context of memory management means usually something else.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org