Closed gakuzzzz closed 3 years ago
@gakuzzzz Thanks for your proposal. It sounds very nice.
Validating arguments "before" creating an object is one of the things YAVI wants to achieve. Actually, YAVI has been providing a similar (more generic?) solution called "Argument Validator" since 0.3.0.
Arguments1Validator<String, Email> validator = ArgumentsValidatorBuilder
.of(Email::new)
.builder(b -> b
._string(Arguments1::arg1, "email",
c -> c.notBlank().lessThanOrEqual(128).email()))
.build();
Validated<Email> emailValidated = validator.validateArgs("foo@example.com");
// or
Email email = validator.validated("foo@example.com");
but your proposal looks simpler and more convenient for a single argument factory method.
I'm wondering if we could leverage the existing ArgumentNValidator
to the proposal and if you should support arguments with numbers greater than 1.
Wow, I hadn't figured ArgumentNValidator
out.
It's sounds nice!
Maybe what we need are combinators for ArgumentNValidator
.
It would be nice to provide combinators that allow Validator
to return Argument1Validator
and ArgumentNValidator
to be composited similarly to Validated#combine
, as follows.
public class Validator<T> implements ValidatorSubset<T> {
...
public <T2> Arguments1Validator<T, T2> andThen(Function<? super T, ? extends T2> f) {
...
}
public <T2> Arguments1Validator<T2, T> compose(Function<? super T2, ? extends T> f) {
...
}
public final class ArgumentValidators {
...
public static <A1, A2, X, Y, Z> Argument2Validator<A1, A2, Z> combine(
Argument1Validator<? super A1, ? extends X> av1,
Argument1Validator<? super A2, ? extends Y> av2,
BiFunction<? super X, ? super Y, ? extends Z> f
) {
...
}
Then, we can build the ContactInfo
validator as follows.
Arguments1Validator<String, Email> emailValidator = ValidatorBuilder.<String>of()
.constraint(s -> s, "email", c -> c.notBlank().lessThanOrEqual(128).email())
.build()
.andThen(Email::new);
Arguments1Validator<String, PhoneNumber> phonNumValidator = ValidatorBuilder.<String>of()
.constraint(s -> s, "phoneNumber", c.notBlank().lessThanOrEqual(16).pattern("[0-9\\-]+"))
.build()
.andThen(PhoneNumber::new);
Arguments2Validator<String, String, ContactInfo> contactInfoValidator =
ArgumentValidators.combine(emailValidator, phonNumValidator, ContactInfo::new);
Validated<ContactInfo> result =
contactInfoValidator.validateArgs("maki@example.com", "0120-3456-7890);
Probably it is a bit tricky to build Arguments2Validator<String, String, ContactInfo>
using ArgumentsValidatorBuilder
and nested
with the current mechanism. However, this approach makes it easier to build it.
ValidatorBuilder.<String>of().constraint(s -> s
is a bit boilerplate.
It might be useful to have a short hand like the following?
Arguments1Validator<String, Email> emailValidator = StringValidator
.of("email", c -> c.notBlank().lessThanOrEqual(128).email())
.andThen(Email::new);
Great ideas. Overall agree.
public class Validator<T> implements ValidatorSubset<T> { ... public <T2> Arguments1Validator<T, T2> andThen(Function<? super T, ? extends T2> f) { ... } public <T2> Arguments1Validator<T2, T> compose(Function<? super T2, ? extends T> f) { ... }
This part needs to be carefully designed.
I'd like to avoid a cyclic dependency on am.ik.yavi.core
and am.ik.yavi.arguments
.
That's for sure. I'd like to avoid the cyclic dependency.
It might be enough for ArgumentsNValidator
to have a map
/ contramap
and the StringValidator.of
short hand returns Arguments1Validator
.
@gakuzzzz I'm working on this enhancement.
Do you think it make sense forArgumentsNValidator
(N
>= 2) to implement contramap
?
I don't think that contramap
on ArgumentsNValidator
(N >= 2) is necessary at first release.
It's probably limited in YAVI use cases, and there's no support for tuples, etc.
If we find an important use case later, we can implement it then.
@gakuzzzz ArgumentsN
will work like a Tuple.
Do you think it makes sense to create contramap
that returns Arguments1Validator
to ArgumentsNValidator
like this?
default <A> Arguments1Validator<A, X> contramap(Function<? super A, ? extends ArgumentsN<A1, A2, ..., An>> mapper) {
return (a, locale, constraintGroup) -> {
final ArgumentsN<A1, A2, ..., An> args = mapper.apply(a);
return ArgumentsNValidator.this.validate(args.arg1(), args.arg2(), ..., args.argN(), locale, constraintGroup);
};
}
This seems to cover the use case I've wanted to solve for a long time: "validate ServletRequest parameters before creating an object".
Arguments2Validator<String, String, Name> nameValidator = ...;
Arguments1Validator<HttpServletRequest, Name> requestValidator = nameValidator
.contramap(req -> Arguments.of(req.getParameter("firstName"), req.getParameter("lastName")));
Validated<Name> nameValidated = requestValidator.validate(servletRequest);
My question is, can mapping ArgumentsNValidator<A1, A2, ..., An, X>
to Arguments1Validator<A, X>
be called contramap
?
Work in progress. Looks pretty cool.
I see, it's good that ArgumentN
can be treated like tuples.
I was hesitant because I felt like I needed to prepare all combinations( ex. BiFunction<Argument5<...>, Argument3<...>, Argument2<...>>
), but I thought it would be very useful if I could use contramap
to fix the input to one and the output to ArgumentN
.
As you said, it would be possible to split and perform validation from a single ServletRequest or Json.
My question is, can mapping ArgumentsNValidator<A1, A2, ... , An, X> to Arguments1Validator<A, X> be called contramap ?
Surely it would be better to use a different name. One name that comes to mind is andThen
/ compose
, where the validator is considered a function (andThen
corresponds to map
and compose
to contramap
), but compose
also has a broad meaning, so I'm torn.
Do you mean
ArgumentsNValidator#map
-> ArgumentsNValidator#andThen
ArgumentsNValidator#contramap
-> ArgumentsNValidator#compose
? https://github.com/making/yavi/commit/72d7eece2ec5ee7fe2b7fdd23fb318f363ef447d
Like this?
Do you mean
- ArgumentsNValidator#map -> ArgumentsNValidator#andThen
- ArgumentsNValidator#contramap -> ArgumentsNValidator#compose
?
yes!!!
Hmm, looking at the Map
example above, it looks like ArgumentValidators.combine
may also have two ways to combine in other directions.
like following
// first example
public static <A1, A2, X, Y, Z> Argument2Validator<A1, A2, Z> combine(
Argument1Validator<? super A1, ? extends X> av1,
Argument1Validator<? super A2, ? extends Y> av2,
BiFunction<? super X, ? super Y, ? extends Z> f
) {
...
}
// a new combine way
public static <A, X, Y, Z> Argument1Validator<A, Z> map2(
Argument1Validator<? super A, ? extends X> av1,
Argument1Validator<? super A, ? extends Y> av2,
BiFunction<? super X, ? super Y, ? extends Z> f
) {
...
}
With this, the above validator for creating a Person from a Map can be created as follows.
final Argument1Validator<String, Email> emailValidator = ...
final Argument1Validator<Map<String, Object>, Email> mapEmailValidator = emailValidator
.compose(map -> (String) map.get("email"));
final Argument1Validator<Integer, Age> ageValidator = ...
final Argument1Validator<Map<String, Object>, Age> mapAgeValidator = ageValidator
.compose(map -> (Integer) map.get("age"));
final Argument1Validator<Map<String, Object>, Person> mapPersonValidator =
ArgumentValidators.map2(mapEmailValidator, mapAgeValidator, Person::new);
final Map<String, Object> params = Map.of("email", "foo@example.com", "age", 100);
final Validated<Person> person = mapPersonValidator.validated(params);
That's interesting.
Why ArgumentValidators.map2
instead of ArgumentValidators.compose
?
ArgumentNValidator#compose
and ArgumentValidators.map2
are different operations.
They are not substitutes.
Also, in the first example, ArgumentValidators.combine
should be named something like ArgumentValidators.split2
to contrast with map2
. (ref twitter log https://twitter.com/hexirp_prixeh/status/1400075567186149376 )
public static <A1, A2, R1, R2, X> Argument2Validator<A1, A2, X> split2(
Argument1Validator<? super A1, ? extends R1> av1,
Argument1Validator<? super A2, ? extends R2> av2,
BiFunction<? super R1, ? super R2, ? extends X> f
) { ... }
public static <A1, A2, A3, R1, R2, R3, X> Argument3Validator<A1, A2, A3, X> split3(
Argument1Validator<? super A1, ? extends R1> av1,
Argument1Validator<? super A2, ? extends R2> av2,
Argument1Validator<? super A3, ? extends R3> av3,
Function3<? super R1, ? super R2, ? super R3, ? extends X> f
) { ... }
...
public static <A, R1, R2, X> Argument1Validator<A, X> map2(
Argument1Validator<? super A, ? extends R1> av1,
Argument1Validator<? super A, ? extends R2> av2,
BiFunction<? super R1, ? super R2, ? extends X> f
) { ... }
public static <A, R1, R2, R3, X> Argument1Validator<A, X> map3(
Argument1Validator<? super A, ? extends R1> av1,
Argument1Validator<? super A, ? extends R2> av2,
Argument1Validator<? super A, ? extends R3> av3,
Function3<? super R1, ? super R2, ? super R3, ? extends X> f
) { ... }
...
Having both split2
and map2
would allow us to choose between the two.
final Argument1Validator<String, Email> emailValidator = ...
final Argument1Validator<Integer, Age> ageValidator = ...
final Argument2Validator<String, Integer, Person> personValidator =
ArgumentValidators.split2(emailValidator, ageValidator, Person::new);
final Argument1Validator<Map<String, Object>, Person> mapPersonValidator =
personValidator.compose(map -> Arguments.of(
(String) map.get("email"),
(Integer) map.get("age")
));
final Map<String, Object> params = Map.of("email", "foo@example.com", "age", 100);
final Validated<Person> person = mapPersonValidator.validated(params);
final Argument1Validator<String, Email> emailValidator = ...
final Argument1Validator<Map<String, Object>, Email> mapEmailValidator = emailValidator
.compose(map -> (String) map.get("email"));
final Argument1Validator<Integer, Age> ageValidator = ...
final Argument1Validator<Map<String, Object>, Age> mapAgeValidator = ageValidator
.compose(map -> (Integer) map.get("age"));
final Argument1Validator<Map<String, Object>, Person> mapPersonValidator =
ArgumentValidators.map2(mapEmailValidator, mapAgeValidator, Person::new);
final Map<String, Object> params = Map.of("email", "foo@example.com", "age", 100);
final Validated<Person> person = mapPersonValidator.validated(params);
Thanks.
Is N
of mapN
and splitN
necessary?
From overloading perspective, N
looks redundant and just map
and split
look ok/
Since map
and map2
have very different semantics, it seems better to unify them with mapN
if we want to overload them into one. (The same goes for splitN
).
FYI https://typelevel.org/cats/typeclasses/applicative.html#syntax
got it, thanks
Is the following code what you intended?
final StringValidator<Email> emailValidator = StringValidatorBuilder
.of("email", c -> c.notBlank().email()).build().andThen(Email::new);
final IntegerValidator<Age> ageValidator = IntegerValidatorBuilder
.of("age", c -> c.greaterThanOrEqual(0)).build().andThen(Age::new);
final Arguments1Validator<Map<String, Object>, Email> mapEmailValidator = emailValidator
.compose(map -> (String) map.get("email"));
final Arguments1Validator<Map<String, Object>, Age> mapAgeValidator = ageValidator
.compose(map -> (Integer) map.get("age"));
final Arguments2Validator<String, Integer, Person> personValidator = ArgumentsValidators
.split2(emailValidator, ageValidator, Person::new);
final Person person1 = personValidator.validate("foo@example.com", 100)
.orElseThrow(ConstraintViolationsException::new);
System.out.println(person1);
final Arguments1Validator<Map<String, Object>, Person> mapPersonValidator = ArgumentsValidators
.map2(mapEmailValidator, mapAgeValidator, Person::new);
final Person person2 = mapPersonValidator.validate(new HashMap<String, Object>() {
{
put("email", "foo@example.com");
put("age", 100);
}
}).orElseThrow(ConstraintViolationsException::new);
System.out.println(person2);
@gakuzzzz I'd like to support method chain pattern as well
How does this look like?
final StringValidator<Country> countryValidator = StringValidatorBuilder
.of("country", c -> c.notBlank().greaterThanOrEqual(2))
.build()
.andThen(Country::new);
final StringValidator<String> streetValidator = StringValidatorBuilder
.of("street", c -> c.notBlank())
.build();
final StringValidator<PhoneNumber> phoneNumberValidator = StringValidatorBuilder
.of("phoneNumber", c -> c.notBlank().greaterThanOrEqual(8).lessThanOrEqual(16))
.build()
.andThen(PhoneNumber::new);
final Arguments1Validator<Map<String, String>, Country> mapCountryValidator = countryValidator
.compose(map -> map.get("country"));
final Arguments1Validator<Map<String, String>, String> mapStreetValidator = streetValidator
.compose(map -> map.get("street"));
final Arguments1Validator<Map<String, String>, PhoneNumber> mapPhoneNumberValidator = phoneNumberValidator
.compose(map -> map.get("phoneNumber"));
final Arguments3Validator<String, String, String, Address> addressValidator = countryValidator
.split2(streetValidator)
.split3(phoneNumberValidator)
.apply(Address::new);
final Arguments1Validator<Map<String, String>, Address> mapAddressValidator = mapCountryValidator
.map2(mapStreetValidator)
.map3(mapPhoneNumberValidator)
.apply(Address::new);
If this makes sense, I'll make following changes
ArgumentsValidators.mapN(Arguments1Validator ...., FunctionN)
-> ArgumentsValidators.mapN(Arguments1Validator ....).apply(FunctionN)
ArgumentsValidators.splitN(Arguments1Validator ...., FunctionN)
-> ArgumentsValidators.splitN(Arguments1Validator ....).apply(FunctionN)
to align the same interface as Validations.combine(Validation...).apply(FunctionN)
Is the following code what you intended?
yes!!!
I'd like to support method chain pattern as well
I agree with supporting method chains. 👍
As a matter of fact, it can use splitN(...) . apply(FunctioN)
to splitN(...) .apply(FunctionN)
, which is more appropriate in the original sense.
Since split
is a method what takes a -> x
and b -> y
and turns them into (a, b) -> (x, y)
.
However, mapN(... , FunctioN)
to mapN(...).apply(FunctionN)
is semantically different, so it's not a map
anymore, isn't it...
mmm...
I'm asking on twitter if there is a good naming. https://twitter.com/gakuzzzz/status/1400417296972062720
Sounds good 👍 will do
So, If mapN
method no longer exists, is it okay if splitN
method is simply split
method?
I'm sure you're right, I think split
will be fine there.
The current
Validator<T>
requires an instance of typeT
to use thevalidate
method. This must allow for the existence ofT
instance in an invalid state.This Email example is a good example. It provides a factory method to prevent creation with invalid strings, but the constructor is exposed to allow users to instantiate it with invalid values.
However, modern general practice recommends that classes should not be instantiated with invalid values.
If the constructor of
Email
is designed to throw a runtime exception when an invalid value is passed, it is not possible to define aValidator<Email>
.So, how about making it possible to separate the type of the value received by the
Validator
from the type of the result?For example, the following.
With such an interface, it is possible to define
EssentialValidator<String, Email>
as follows, even if Email is designed to throw an exception in its constructor.This approach is to define validators of a small type, and then combine them to create a large-structure validator. However, YAVI is originally designed to create a large-structure validator by extracting parts of a large-structure object and writing constraint after constraint. Therefore, I am not sure if this proposal is really necessary as a use case of YAVI or not.