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

Support overriding constraint messages (i18n support) #21

Open
pjean opened this issue May 20, 2014 · 19 comments
Open

Support overriding constraint messages (i18n support) #21

pjean opened this issue May 20, 2014 · 19 comments

Comments

@pjean
Copy link

pjean commented May 20, 2014

It will be nice to have i18n support or at least a way to customize constraints messages.

@holograph
Copy link
Contributor

Indeed :-) I'm still trying to decide how to best approach this -- one possibility is to change the architecture so that constraint messages are not necessarily strings, and can be overridden. Question is, what would the ideal syntax look like? A few ideas:

  1. p.firstName is notEmpty orElse AppSpecificMessages.firstNameMissing
  2. p.firstName is notEmpty -> AppSpecificMessages.firstNameMissing
  3. p.firstName is( notEmpty, AppSpecificMessages.firstNameMissing )

Where AppSpecificMessages is an object and firstNameMissing is a val of some type T. 2 and 3 are progressively easier to implement but are not as semantically clear as 1.

One concern is that I'd like to keep the constraint processing logic type-safe, which would mean having some notion of a constraint "base enumeration type" known to the validator in advance. Either that, or the constraint can simply be Any and let the processing logic deal with the fallout.

If you have a real use-case in mind, this would really help designing a proper API for this feature, and I'm very much open to ideas :-)

@pjean
Copy link
Author

pjean commented May 27, 2014

Yep, the first idea is the best and like you said, it's better to keep the constraint processing logic type-safe. We must be able to format the messages too in order to specify the wrong value in the message for example.
Another feature that will be great, it's a conditional validation like that :
p.firstName is notEmpty if p.middleName is notEmpty orElse ...

@holograph
Copy link
Contributor

if is a keyword so is not really usable in this context, but it might be possible to have a similar-but-sane syntax. I'll finish the earlier items in the pipeline (Scala 2.11 for one) and see what I can do. Pull requests are always welcome as well :-)

@holograph holograph added this to the 0.5 milestone Jul 23, 2014
@holograph holograph self-assigned this Jul 23, 2014
@hussachai
Copy link

IMO, I think you can do it simply by putting the message key and args instead of message
Change

case class RuleViolation( value: Any, constraint: String, description: Option[ String ] )

to something like

case class RuleViolation( value: Any, constraintKey: String, contraintArgs: Seq[Any], description: Option[ String ] )

Doing this way we can use the existing i18n that most MVC frameworks provide.
For example
I think it's better to get this

RuleViolation(7,"validation.size.between",Seq("firstName", 7, 1, 5), Some(firstName))

instead of this

RuleViolation(7,"has size 7, expected between 1 and 5",Some(firstName))

Would be nice to have field name as the first item in Seq because developer can customize it

validation.size.between={0} must be between {2} and {3} but {1}

@holograph
Copy link
Contributor

I'm not very fond of this idea, because:

  1. There's no sensible default behavior (unless a specific framework is baked in as the default);
  2. It's very hard to document properly, in a way that does not violate the principle of least surprise. For instance, Spring Validation's documentation makes way too many assumptions about the reader, and in my case I still don't get exactly how error codes/messages work there.

One way of possibly doing this is to provide a cake -- the message shape is determined by the desired backend (so you can import com.wix.accord.simple._ or com.wix.accord.someframework._ etc.), but that has complexity and performance ramifications I'm not very keen on.

@hussachai
Copy link

Thank you for replying
Sorry I have different opinion. If the framework has its own i18n database, it will be
difficult for developer to deal with how each framework manages its own i18n database.
It requires more work to incorporate all frameworks (validation, form binding, template, exception, etc.) i18n system and transform them to desired format. More importantly, i18n information will be attached to framework instead of storing somewhere else that can be reused. If one day we decide to replace framework X with framework Y, we have to create i18n and find the way to deal with new framework i18n again. That's why I purposed that simple idea. Just leave that responsibility to developer to decide how to deal with message key and args.
Having message key and args is not the best way to do i18n but it's the best way as of now IMO
to manage i18n database in one place.

I'm from Java and Spring messageSource is one of my favorite in Spring. This may explain
why you and I like different things. :)
In my case, right now I'm trying to define the generic form-feedback-error-message in Play framework. play-json macro returns the message key and args but I don't like how JSON Reads/Writes combinators work because I don't want to hard code field name. So, I just use Formats for case class serializer/de-serializer and I want some validation frameworks to work on case class. I want to generalize json-case class binding message and validation error message together. I'm doing PoC and this framework got my attention. Please keep up doing good work :D

@JonathanStevensFN
Copy link

I am interested in this one enhancement. I would like to be able to build my own Failure since I need some different information from a failed validation.

Jono

@holograph
Copy link
Contributor

I'm exploring two possible approaches to generalizing constraint types. In both cases the idea is for the existing, string-based constraints to be moved to wix.accord.simple (with wix.accord providing a backwards-compatible shim for a single milestone release), and users able to specify their own representation. These approaches are:

  1. Cake (pattern, that is): the entirety of the Accord library is placed under a single trait, intended to be exposed via a singleton object. Constraint is a type member, and a Domain icing-on-the-cake trait enforces that all constraints for the built-in library are actually defined. You can see the ongoing work on the free-constraints branch.
  2. Implicits + dependent method types. This leverages some of @propensive's ideas on modes, so that the Accord library and APIs are agnostic to the constraint type, and all type-specific information is filled in at compile-time via typeclasses and implicit resolution. I'm still experimenting here, but this will likely require a significant overhaul of the result model and many public APIs (particularly the DSL), but may end up cleaner overall. You can see the ongoing work on the free-constraints-implicits branch.

As it stands, both of these approaches appear to be tremendously complicated from an API standpoint; I'll give it a few more hours, but if either approach doesn't appear to pan out, I'll instead go with a radically simplified model: each validator will define (and include in the resulting violation) its own specific constraint type, deriving from a common root. This will probably necessitate adding a type parameter to the violation model as well as validators themselves, which isn't awful but is certainly something to take into account.

@holograph
Copy link
Contributor

Exactly one month later, and I have something to show for it. The free constraints branch has reached a stable milestone, where all tests pass and things are largely working. In a nutshell, the idea is that the whole Accord cake is now accessible via "domains". A domain defines the type of constraints, and provides constraint generators for core combinators. For example, the legacy, string-based model is called "simple" (have a look at SimpleConstraints to get an idea what implementing a domain is like).

The included example project (before and after migration) reflects most of the required changes when migrating to this new approach. These are:

  • The Accord API is imported via the domain, e.g. import com.wix.accord.simple._;
  • The DSL is made accessible via the domain, e.g. import com.wix.accord.simple.dsl._;
  • Result matchers are now parameterized by their domain, e.g. class MySpec extends Specification with Matchers with ResultMatchers[ simple.type ] { ... };
  • When extending the domain with new validators you need to be aware of the constraint type in flight. There are a number of strategies for generalizing this to any domain, but I'll leave that consideration for if this solution seems to pan out.

@JonathanStevensFN
Copy link

is this feature available on the 0.5 snapshot to play with?

@holograph
Copy link
Contributor

Sorry for the delay -- no, it's experimental enough that I placed it in its own branch, I'm not sure it makes sense to publish this on any repo before I get some feedback. You can very easily build it yourself (clone, switch to free-constraints branch and run sbt publishLocal).

@rpiaggio
Copy link

rpiaggio commented Apr 7, 2016

+1 for this. We really like Accord but this feature has become a necessity for us. BTW, thank you for the great library!

@dsounded
Copy link

dsounded commented Aug 4, 2016

Some way of replacing default messages with the others is really needed

@holograph
Copy link
Contributor

This, I believe, will be the major focus for either the next milestone release (0.7) or, if it progresses nicely enough, round 1.0 off. I have some new notions on how to do this, and will go into more detail later.

@holograph holograph changed the title i18n support Support overriding constraint messages (i18n support) Aug 6, 2016
@holograph holograph modified the milestones: 0.7, 1.0 Dec 25, 2016
@holograph
Copy link
Contributor

Time to seriously tackle this issue. If anyone has additional feedback, now's a good time :-)

@holograph
Copy link
Contributor

Quick brain-dump of current possible directions:

Runtime dispatch

If we give up type safety, the problem becomes very easy -- each combinator determines its own constraint type and uses that. Accord can then provide a very simple rendering framework and it's up to the user to handle the dispatch (or register renderers, a la Jackson modules).

This is the fallback method. Accord's main claim to fame is type safety, and I'd hate to have to give that up.

Cake

See discussion above. I believe this is far too complex to use in practice, requiring users to define their own cake and import it all over the place, and composing cakes becomes an exercise in annoyance. This doesn't jive with Accord's stated design goal of dead-simple APIs.

Type encoding

  1. In order to retain type safety, a combinator must specify its resulting constraints. Let's call this Validator[T, C] where C is the error type.
  2. Validators can be composed, so C must encode multiple possible constraint types. This can possibly be encoded as a phantom type as in type C = C1 with C2... with Cn, and functions that aggregate validations (e.g. com.wix.accord.dsl.validator, Function1.compose, Function1.andThen) can be reimplemented with macros to collect these into an ordered set. Since the result type isn't known at compile type by definition, this necessitates whitebox macros, which is the primary argument against this approach (Dotty should solve this with true disjunction types, but that's quite a ways off).
  3. Validation result (i.e. Failure) must encode the possible constraints, so type Validator[T, C] = T => Result[C].

This gets us to the point where all possible constraint types are encoded in the result; breaking them down into discrete components requires (I believe) yet another macro. A generic render function than can be used to transform a result into the desired domain model (Spring validation, strings, ...) with typeclasses:

trait ConstraintRenderer[C, R] extends (C => R)

trait ResultModel[T] {
  type Context
  type Constraint
  type Result = T

  def newContext: Context
  def addConstraint(context: Context, constraint: Constraint): Unit    // Hand-wavy. Needs to take hierarchy into account
  def finalizeContext(context: Context): T
}

def render[C](result: Result[C])(implicit model: ResultModel[_]): model.Result = macro renderImpl

def renderImpl(...) = {
  // A macro which:
  // 1. Break down C into separate components C1, C2...,CEn
 
  // 2. Generate an implicit requirement for each C as follows:
  val mappers = Map(
    classOf[C1] -> implicitly[ConstraintRenderer[C1, model.Constraint]],
    classOf[C2] -> implicitly[ConstraintRenderer[C2, model.Constraint]],   // ...
    classOf[Cn] -> implicitly[ConstraintRenderer[Cn, model.Constraint]]
  )

  // 3. Generates code to map constraints to target result model
  val context = model.newContext
  result.flatten foreach { violation =>
    val mapper = mappers.get(violation.constraint.getClass).getOrElse(throw new Exception("Safety net: there's a bug in Accord"))
    model.addConstraint(context, mapper(violation.constraint))
  }
  model.finalizeContext(context)
}

The usage site would then be:

case class Person(name: String, age: Int)
case object Person {
  implicit val personValidator = validator[Person] { p =>
    p.name is notEmpty  // Resolves to type C1 = Not[Empty.type]
    p.age must be >= 18  // Resolves to type C2 = BinaryOp[Int]
  }
  // Type of personValidator is Validator[Person, Not[Empty.type] with BinaryOp[Int]]
}

val result = validate(Person("", -6))    // Resolves to Result[Not[Empty.type] with BinaryOp[Int]]

import com.wix.accord.rendering.Text._
// Text would include the following implicits:
// implicit val ResultModel[String]
// implicit def renderNot[C](implicit r: ConstraintRenderer[C, String]]): ConstraintRenderer[Not[C], String]
// implicit def renderBinaryOp[T]: ConstraintRenderer[BinaryOp[T]](implicit r: ConstraintRenderer[T, String]])
// ...

val rendered: String = render(result)

The generated implicit requirement guarantees that renderers for all requisite constraint types are available. I'm rather leery about the whitebox macros, but this is the most promising avenue of research I have.

@holograph holograph modified the milestones: 1.0, 0.7 Jul 9, 2017
@wojtasskorcz
Copy link

wojtasskorcz commented Oct 17, 2017

Just wanted to give +1 here too, it would be really great to have this feature. Or is there any way at all to customize a constraint message (not necessarily compatible with i18n frameworks etc)?

Edit: Nevermind, just reimplemented the EqualTo validator for the time being with a second parameter accepting a custom message.

@mladibejn
Copy link

I really like the feature as well, any news on when it would be released? Thanks

@holograph
Copy link
Contributor

Pinging @noam-almog, a fresh pair of eyes would be invaluable here

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

No branches or pull requests

8 participants