Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for refined #405

Merged
merged 5 commits into from
Feb 6, 2020

Conversation

strokyl
Copy link
Contributor

@strokyl strokyl commented Jan 30, 2020

Hello,
I write this piece of code today.
If I improve it (need way to improve error message when decode fail), would you be interrested in merging it?

refineV[P](v) match {
case Right(refined) => DecodeResult.Value(refined)
//TODO: exploit error
case Left(_) => DecodeResult.InvalidValue(List())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a simple way to construct a validation error out of a String, I guess no.
I guess I will need to derive a validator out of P (predicate).
But can I have a validator on "V Refined P" before V get construct out of P?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use the Validator.Custom, which allows an arbitrary message.

You could also probably define an implicit validator for refined types (where the logic of the validator would run the refined check).

So you would have an implicit custom validator - using refined (for nested refined types; this would be used when deriving codecs for complex datatypes), an an implicit codec for top-level refined types

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to create the custom validation message, maybe there's a way to Show the refined type constraints?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So unfortunatly for the moment, given a validator[T, V] the only way to get a human description out of it is to have a concrete T value that does not respect it.

If I have time, I will try to make a PR to refined to add a description method on the validator trait, because I think it definitly make sense.

Meanwhile, in this PR I used several compromise:

  • Use direct tapir validator when possible (for the moment I did it only for Regexp and NonEmptyString because I am lazy ;) )
  • When no direct tapir validator mapping, generate a custom validator with error being the classname of the refined constraint. But when the "validation/refinition" fail, I replace this really bad errror message with the actual failure from refined.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! That way, this will also end up in the openapi docs. Maybe you could also add this for min/max numbers?

build.sbt Outdated Show resolved Hide resolved
@strokyl strokyl changed the title WIP: add support for refined Add support for refined Jan 31, 2020
@adamw
Copy link
Member

adamw commented Feb 3, 2020

One last thing would be the docs, similarly as in here: https://tapir-scala.readthedocs.io/en/latest/endpoint/customtypes.html#schema-for-cats-datatypes

@strokyl
Copy link
Contributor Author

strokyl commented Feb 5, 2020

Ok so I added the doc.
I think the PR is ready to be merged if you agree.
But I think in the future we can improve all of that by adding more custom binding.

`integration/refined/src/main/scala/sttp/tapir/codec/refined/TapirCodecRefined.scala`.
If you are not satisfied with the validator generated by `tapir-refined`, you can provide an implicit
`RefinedValidatorTranslation[T, P]` in scope using `RefinedValidator.fromPrimitiveValidator' to build it (do not
hesitate to contribute your work).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I like the last line :)

@adamw adamw merged commit daa0a8d into softwaremill:master Feb 6, 2020
@adamw
Copy link
Member

adamw commented Feb 6, 2020

Released in 0.12.20. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants