-
-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use more specific data type when some pgv-rules are present #38
Comments
It looks like we're looking at two options:
(2) is definitely easier - since it doesn't interfere with the parsing logic. Do both options work equally well for you? |
@thesamet Thanks for summing that up. Since i opened the issue, i have encountered the same pattern not only for |
@thesamet what would be nice is to somehow be able to map a set of PGV rules to a scala type: def typeForRule(rules: io.envoyproxy.pgv.validate.FieldRules): Any {
rules match {
.... => NonEmptySet
.... => Email
}
} |
This proposal makes sense, and I think this can make a nice addition to this library. Currently, there is no way for one plugin to change the types emitted by ScalaPB. We would have to come up with a new capability in core ScalaPB that would enable this. This appears to be high effort, and only applicable to a specific user group (scalapb-validate && Cats). It would be great if this work can be sponsored by one of the companies who use it, or someone putting together the necessary design and PRs for discussion. |
For the record, I am happy to announce that Teads will be sponsoring this work! |
Most of the PGV rules can "manually" be encoded/enforced by types by annotating fields with a
This issue was originally targeted at (1), but (2) was introduced in the discussion. In our own use-case:
|
However, the types might not capture all PGV constraints, so |
If I understand correctly the preference is to have the case class constructor call pgv-validate so it is impossible to instantiate directly or through parsing partially invalid objects? This can be done as part of this change. This would be pretty substantial change for other users of scalapb-validate since they'll lose control of when the validation is done. |
That's correct. As a matter of fact, in our trait ScalaPBValidator[ScalaPB] {
self: ScalaPB =>
def validator: Validator[ScalaPB]
validator.validate(self) match {
case Success => ()
case Failure(violations) =>
val description = violations.map(_.getMessage).mkString("\n")
throw new GrpcServiceException(INVALID_ARGUMENT.withDescription(description))
}
} I guess it will depend on the implementation, but what we could loose if we encode a repeated field with NEL would be a clear exception with the violated validations (if the NEL constructor is called first). That's why I brought up the subject of partial validation, but I haven't given it much thoughts yet. |
Do we always want to have |
Good point. At the moment we don't have a case where we need "ordered list without duplicates" semantics, but that's what it really is. Which brings the question of opting-in for |
May be we could also rely on a |
MessageBuilder is a mutable class providing the ability to merge a message from a CodedInputStream into the mutable state, and build an instance of a message. MessageCompanion.newBuilder returns a new Builder initialized with empty values. MessageCompanion.newBuilder(msg) returns a new Builder initialized with values from a given instance of a message. This new functionality allows us to create builders with partially initialized objects that can not be represented by the actual message. See #1013 and scalapb/scalapb-validate#38
In order to support Collections that are quite different than the ones in the standard library, we add a new `collection` option for repeated fields. The collection can reference an `Adapter` type available at runtime that will forward calls for an underlying collection. This helps support types such as cats.data.NonEmptyList, NonEmptySet and NonEmptyMap. For example, those collections don't define `foreach`, and `++` where the rhs is an iterable. For `NonEmptySet` there is even no `size` method. See #1013 and scalapb/scalapb-validate#38
MessageBuilder is a mutable class providing the ability to merge a message from a CodedInputStream into the mutable state, and build an instance of a message. MessageCompanion.newBuilder returns a new Builder initialized with empty values. MessageCompanion.newBuilder(msg) returns a new Builder initialized with values from a given instance of a message. This new functionality allows us to create builders with partially initialized objects that can not be represented by the actual message. See #1013 and scalapb/scalapb-validate#38
In order to support Collections that are quite different than the ones in the standard library, we add a new `collection` option for repeated fields. The collection can reference an `Adapter` type available at runtime that will forward calls for an underlying collection. This helps support types such as cats.data.NonEmptyList, NonEmptySet and NonEmptyMap. For example, those collections don't define `foreach`, and `++` where the rhs is an iterable. For `NonEmptySet` there is even no `size` method. See #1013 and scalapb/scalapb-validate#38
MessageBuilder is a mutable class providing the ability to merge a message from a CodedInputStream into the mutable state, and build an instance of a message. MessageCompanion.newBuilder returns a new Builder initialized with empty values. MessageCompanion.newBuilder(msg) returns a new Builder initialized with values from a given instance of a message. This new functionality allows us to create builders with partially initialized objects that can not be represented by the actual message. See #1013 and scalapb/scalapb-validate#38
In order to support Collections that are quite different than the ones in the standard library, we add a new `collection` option for repeated fields. The collection can reference an `Adapter` type available at runtime that will forward calls for an underlying collection. This helps support types such as cats.data.NonEmptyList, NonEmptySet and NonEmptyMap. For example, those collections don't define `foreach`, and `++` where the rhs is an iterable. For `NonEmptySet` there is even no `size` method. See #1013 and scalapb/scalapb-validate#38
MessageBuilder is a mutable class providing the ability to merge a message from a CodedInputStream into the mutable state, and build an instance of a message. MessageCompanion.newBuilder returns a new Builder initialized with empty values. MessageCompanion.newBuilder(msg) returns a new Builder initialized with values from a given instance of a message. This new functionality allows us to create builders with partially initialized objects that can not be represented by the actual message. See #1013 and scalapb/scalapb-validate#38
In order to support Collections that are quite different than the ones in the standard library, we add a new `collection` option for repeated fields. The collection can reference an `Adapter` type available at runtime that will forward calls for an underlying collection. This helps support types such as cats.data.NonEmptyList, NonEmptySet and NonEmptyMap. For example, those collections don't define `foreach`, and `++` where the rhs is an iterable. For `NonEmptySet` there is even no `size` method. See #1013 and scalapb/scalapb-validate#38
MessageBuilder is a mutable class providing the ability to merge a message from a CodedInputStream into the mutable state, and build an instance of a message. MessageCompanion.newBuilder returns a new Builder initialized with empty values. MessageCompanion.newBuilder(msg) returns a new Builder initialized with values from a given instance of a message. This new functionality allows us to create builders with partially initialized objects that can not be represented by the actual message. See #1013 and scalapb/scalapb-validate#38
In order to support Collections that are quite different than the ones in the standard library, we add a new `collection` option for repeated fields. The collection can reference an `Adapter` type available at runtime that will forward calls for an underlying collection. This helps support types such as cats.data.NonEmptyList, NonEmptySet and NonEmptyMap. For example, those collections don't define `foreach`, and `++` where the rhs is an iterable. For `NonEmptySet` there is even no `size` method. See #1013 and scalapb/scalapb-validate#38
MessageBuilder is a mutable class providing the ability to merge a message from a CodedInputStream into the mutable state, and build an instance of a message. MessageCompanion.newBuilder returns a new Builder initialized with empty values. MessageCompanion.newBuilder(msg) returns a new Builder initialized with values from a given instance of a message. This new functionality allows us to create builders with partially initialized objects that can not be represented by the actual message. See #1013 and scalapb/scalapb-validate#38
My bad, indeed I realized after writing this that we cannot operate on unknown ids if we want to be able capture arbitrary option values with a human-readable path syntax
I just tried your recursive fix (with a local build of scalapb-validate with Scala PB -preview10), and unfortunately it's not enough as the same problem happens for regular (non-package.proto) files which include neither |
In preview11, |
Happy to report that with a local build of ScalaPB/scalapb-validate (preview11 + my last fix), we are utilizing all the features made available for this in our main project:
From our side, it looks like the preview branch is ready to be merged! |
MessageBuilder is a mutable class providing the ability to merge a message from a CodedInputStream into the mutable state, and build an instance of a message. MessageCompanion.newBuilder returns a new Builder initialized with empty values. MessageCompanion.newBuilder(msg) returns a new Builder initialized with values from a given instance of a message. This new functionality allows us to create builders with partially initialized objects that can not be represented by the actual message. See #1013 and scalapb/scalapb-validate#38
In order to support Collections that are quite different than the ones in the standard library, we add a new `collection` option for repeated fields. The collection can reference an `Adapter` type available at runtime that will forward calls for an underlying collection. This helps support types such as cats.data.NonEmptyList, NonEmptySet and NonEmptyMap. For example, those collections don't define `foreach`, and `++` where the rhs is an iterable. For `NonEmptySet` there is even no `size` method. See #1013 and scalapb/scalapb-validate#38
This changes introduces the ability to write "-name" as a preprocessor name to have it excluded at the file-level in the case where it is inherited from the package level. For scalapb/scalapb-validate#38
MessageBuilder is a mutable class providing the ability to merge a message from a CodedInputStream into the mutable state, and build an instance of a message. MessageCompanion.newBuilder returns a new Builder initialized with empty values. MessageCompanion.newBuilder(msg) returns a new Builder initialized with values from a given instance of a message. This new functionality allows us to create builders with partially initialized objects that can not be represented by the actual message. See #1013 and scalapb/scalapb-validate#38
In order to support Collections that are quite different than the ones in the standard library, we add a new `collection` option for repeated fields. The collection can reference an `Adapter` type available at runtime that will forward calls for an underlying collection. This helps support types such as cats.data.NonEmptyList, NonEmptySet and NonEmptyMap. For example, those collections don't define `foreach`, and `++` where the rhs is an iterable. For `NonEmptySet` there is even no `size` method. See #1013 and scalapb/scalapb-validate#38
This changes introduces the ability to write "-name" as a preprocessor name to have it excluded at the file-level in the case where it is inherited from the package level. For scalapb/scalapb-validate#38
Released preview14. This release generalizes again the DSL syntax. The changes are:
|
We have successfully integrated preview14 and started leveraging the more generic syntax to expand the proto |
Fantastic! For ScalaPB, I merged the |
I am really happy to announce that we have just deployed our app with the official release version 🚀 |
MessageBuilder is a mutable class providing the ability to merge a message from a CodedInputStream into the mutable state, and build an instance of a message. MessageCompanion.newBuilder returns a new Builder initialized with empty values. MessageCompanion.newBuilder(msg) returns a new Builder initialized with values from a given instance of a message. This new functionality allows us to create builders with partially initialized objects that can not be represented by the actual message. See #1013 and scalapb/scalapb-validate#38
In order to support Collections that are quite different than the ones in the standard library, we add a new `collection` option for repeated fields. The collection can reference an `Adapter` type available at runtime that will forward calls for an underlying collection. This helps support types such as cats.data.NonEmptyList, NonEmptySet and NonEmptyMap. For example, those collections don't define `foreach`, and `++` where the rhs is an iterable. For `NonEmptySet` there is even no `size` method. See #1013 and scalapb/scalapb-validate#38
MessageBuilder is a mutable class providing the ability to merge a message from a CodedInputStream into the mutable state, and build an instance of a message. MessageCompanion.newBuilder returns a new Builder initialized with empty values. MessageCompanion.newBuilder(msg) returns a new Builder initialized with values from a given instance of a message. This new functionality allows us to create builders with partially initialized objects that can not be represented by the actual message. See #1013 and scalapb/scalapb-validate#38
In order to support Collections that are quite different than the ones in the standard library, we add a new `collection` option for repeated fields. The collection can reference an `Adapter` type available at runtime that will forward calls for an underlying collection. This helps support types such as cats.data.NonEmptyList, NonEmptySet and NonEmptyMap. For example, those collections don't define `foreach`, and `++` where the rhs is an iterable. For `NonEmptySet` there is even no `size` method. See #1013 and scalapb/scalapb-validate#38
We are using scalapb, scalapb-validate and cats, we facilitate the manipulation of the generated classes, we would like to take advantage of both
For example in this case, we would like to expose a the items as a
cats.NonEmptyList
and here a
cats.NonEmptySet
Since the validation is made on an instantiated object, the use of scalapb customization
(scalapb.field).collection_type
seems unadapted.May be we could generate somehow a method that return the appropriate specific data type:
Is there a way to do that?
The text was updated successfully, but these errors were encountered: