10:08 a.m.
On 7 Nov 2016, at 23:42, Hendrik Ebbers <hendrik.ebbers(a)me.com>
wrote:
And this is not the only example. We already have some classes in JavaSE that will end in
such problems:
The basic Property class in JavaFX can be used like this:
private Property<String> myStringProperty;
Based on the current approach we should add a constraint annotation like this:
private Property<@NotNull String> myStringProperty;
But next to the property class JavaFX contains the class StringProperty that is a
specific implementation of „Property<String>„. If we want to use this class we can
not add a annotation to a generic type since we don’t have such a generic type. Here we
need to do it this way:
@NotNull
private StringProperty myStringProperty2;
And again we have 2 different approaches:
private Property<@NotNull String> myStringProperty;
@NotNull
private StringProperty myStringProperty2;
Based on this I do not think that it’s a good idea to use the constraint annotation for
generic types to validate the content of a wrapper.
General comment on you entire email.
I think you are throwing the baby out with the bath water. Granted, more complicated cases
don’t scale as nicely as we would hope but let’s get back tot he basics. 95%+ of this
feature usage will be done for collections and maps. Based on this, I’d be a bit unhappy
at giving up parameterized type use location for landing that annotations. This is sooooo
natural.
Our job is to make the common case simple and ideally make it scale in a not too ugly way
and not close doors to the most complex use cases. And if one thing has to break, we can
drop some of the less common cases.
More specifically to this question, the proposal favored the use of parameterized type use
but offers a backup plan for when the parameterized type use location is not present (like
specialized subclasses of Java FX). Java FX is a clear cut as the wrapper is purely
technical and all of the constraints do apply to the wrapped value.
But everywhere we have a parameterized type use, we should have the app developer make use
of them instead of using the @ConstraintAppliesTo workaround.
To be true I do not have the ultimative solution for this problem.
Currently I trying to add some metadata to the annotations to define it’s goal. Here is an
example:
@NotEmpty
private List<String> names1;
@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS)
private List<String> names2;
@NotEmpty
@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS)
private List<String> names3;
Here the „applyTo“ value was added to the @NotEmpty annotation (like bean validation
already does with the groups). By using this value a developer can specify if he wants to
validate the value of the field or its internals. By doing so „names1“ is valid if the
list contains at leas 1 element, „names2“ is valid if all elements in the list have at
least a length of 1 (an empty list is allowed) and in „names3“ the list must contain at
least one element and all elements must have a length > 0.
A similar approach is already defined in the proposal and I understand why this is not
the perfect solution since you need to define it whenever you do not want to valide a
plain value:
@NotNull(applyTo = ConstraintsApplyTo.INTERNALS)
private StringProperty myStringProperty2;
Extracting this information to an additional (optional) annotation will be a problem
since this can end in such an expression:
@NotEmpty
@ConstraintsApplyTo(CONTAINED_VALUES)
@NotEmpty
private List<String> names3;
By doing so you can not define the same constraints type for the container and for the
containing value. In the example above the same is working.
We did explore per annotation location overriding. This is a question in the initial
proposal. The reason it did not make the first cut is as you have seen:
- you need a lot of information on side that it not trivial to digest
- each existing constraint annotation will have to be changed to add these new attributes
- on that front, the spec has declared that attribute names starting with “valid” are
reserved for the spec, so your proposal would have to have attributes prefixed with
“valid” not to break existing code
- at the end of the day, the feature is really for code using type use, so we are twisting
it for edge cases
But I am very open to the idea of doing per annotation location overriding if people think
that they are an important feature.
Next to the „applyTo“ value all the constraint annotations needs
additional information to really validate all the given examples. In addition a plugin
must be present that defines how we can get the internal value(s) of a class (in this case
„StringProperty“).
Here are some code snippets that use additional information in the annotations to define
the validation:
@NotEmpty
@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS)
@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 2)
@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 3)
private List<List<List<StringProperty>>> spooky;
@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, valueContainerClass = RealValue.class)
private RealValue<String> myValue;
The „applyToInternalDepth“ value is used to define how often the validator should get the
internal value (depth). In the given example „@NotEmpty(applyTo =
ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 3)“ would validate the content of the
„StringProperty“. In addition we need some information for the container if the container
implements several interfaces. This could be defined in the annotation, too. In the
example this is defined by the „valueContainerClass“ value.
Hopefully we can converge on my idea not to have to add valueContainerClass (see the other
emails). But you do need a way to identify the correct type parameter targeted. This is
equivalent to the @ExtractedValue which today has typeParameterHost and either
typeParameterIndex or typeParameterName to uniquely identify them.
On the depth approach, your example would be more easily rewritten as
@NotEmpty List<@NotEmpty List<@NotEmpty List<@NotEmpty
StringProperty>>> spooky;
which clarifies everything. But let’s assume we have a type
ListOfListOfListOfStringProperty type. do we really want to support
@NotEmpty
@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS)
@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 2)
@NotEmpty(applyTo = ConstraintsApplyTo.INTERNALS, applyToInternalDepth = 3)
private ListOfListOfListOfStringProperty spooky;
or even
@NotEmpty
// support nested levels by positioning ConstraintsApplyTo at the right level
private List<@NotEmpty List<@NotEmpty @NotEmpty(applyTo =
ConstraintsApplyTo.CONTAINED_VALUES) ListOfStringProperty>> spooky;
Or do we throw the towel and say that nested levels are not supported for non
parameterized type use locations? What I am realizing now is that this option is available
essentially to support containers that do not offer parameterized type. And in a previous
email, I am questioning that use case.
Based on this a constraint annotation needs this additional fields:
ConstraintsApplyTo applyTo() default ConstraintsApplyTo.VALUE; // VALUE or INTERNALS
int applyToInternalDepth() default 1;
Class<?> valueContainerClass() default Object.class;
As I said I don’t have a good solution :( But I think that based on the given issues
simply adding annotations to the generic types isn’t the best idea.
I think it’s quite important to have support for all the wrapper classes. Validation
collections and (JavaFX) properties will be a big benefit.