You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Jan Vermeulen <ja...@isencia.com> on 2007/06/19 13:54:21 UTC
Problem with latest changes in FormComponent:validate()
The code below is the actual code of FormComponent:validate()
public final void validate()
{
validateRequired();
if (isValid())
{
convert();
if (isValid() && getConvertedInput() != null)
{
validateValidators();
}
}
}
The code in bold has been added. Problem is that we are using a validator
(that ends up consulting a Hibernate validator annotation) to determine if a
field is required for an entity.
But with the added code, this validator never gets called ! If we already
passed through an isValid(), could you not suppose that the extra check is
not needed ? Or should it not be part of the isValid() logic, in a way that
is overridable (isValid() is final) ?
Jan.
--
View this message in context: http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11193140
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: Problem with latest changes in FormComponent:validate()
Posted by Jan Vermeulen <ja...@isencia.com>.
I understand, and yes it makes sense. Validation, and the order in which it
should be invoked, is a complex theme, and we have been struggling with it
for a while too.
We make a distinction between syntactic and semantic (business) validation:
syntactic validation should happen before conversion, to prevent impossible
conversions (f.i. a syntax error in a date string). But semantic validation
should happen after conversion, as it is supposed to work on the converted
type (f.i. is a data out of range). Now, we consider all Hibernate
validation annotations as semantic validation: they are defined on the
entity. In the case of the @NotEmpty validation, you can perfectly represent
absence of value by a null (using the object wrappers for native types), so
the converter should not have any problems with that.
Does that make sense ?
Jan.
--
View this message in context: http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11208700
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: Problem with latest changes in FormComponent:validate()
Posted by Igor Vaynberg <ig...@gmail.com>.
what version are you using?
RequiredValidator is gone in 1.3, let me try to explain the reason:
warning, below is a brain dump, so tread softly
it has to do with type-conversion and where it fits in the workflow of
validating the component
we want validators to work on properly converted types. so when you write a
MinIntValidator you should expect validatable.getvalue() to return an
integer, not a string that you then have to convert to the integer yourself.
the typical workflow for validating a form component is
check required, perform type conversion, run the rest of validators
the reason it has to be that way is that the only valid place to perform
type conversion is after you have checked if the component is required or
not, because you dont want to try to convert an empty string to a typed
object all the time - wheter it is valid or not.
if we do not do this then every validator you write has to do two things
before it can actually get to its job: check if the value is not-null and
perform any required type conversion. so every validator has to cope with
type conversion errors, etc.
we wanted to factor out type conversion out of validators, which also meant
factoring out the required check as well.
makes sense?
-igor
On 6/19/07, Jan Vermeulen <ja...@isencia.com> wrote:
>
>
> Problem with all these solutions, is that we have to differentiate between
> 'RequiredValidators' and other validators, which means somehow we have to
> know which ones to call when. Because now, there is just a list of
> validators, added to the component, and they get called by Wicket. What
> I'm
> trying to say is that checking if a field is empty or not can be just a
> validator task. Now, we can no longer simply plug that in as just another
> validator.
>
> I understand that it is handy not to have to check if a value is null, but
> we're paying a price for this that might be higher than the gain ?
>
> Jan.
>
>
> igor.vaynberg wrote:
> >
> > or instead,
> >
> > maybe make that hibernatevalidator also be a behavior, and simply toggle
> > component's setrequired in onattach
> >
> > -igor
> >
> >
> > On 6/19/07, Igor Vaynberg <ig...@gmail.com> wrote:
> >>
> >> i dont know about that. isvalid's contract is that it only returns
> false
> >> if there are error messages reported against the component.
> >>
> >> i think this would be simple enough
> >>
> >> class hibernatetextfield extends textfield {
> >> valid() {
> >> if (hibernaterequired()) {
> >> if (Strings.isEmpty(getInput()[0])) {
> >> error(new ValidationError().addMessageKey("Required"));
> >> return;
> >> }
> >> }
> >> super.validate();
> >> }
> >>
> >> -igor
> >
>
> --
> View this message in context:
> http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11200959
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>
Re: Problem with latest changes in FormComponent:validate()
Posted by Jan Vermeulen <ja...@isencia.com>.
I'll take a look at it. Thanks.
Jan.
--
View this message in context: http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11411421
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: Problem with latest changes in FormComponent:validate()
Posted by Ryan Sonnek <ry...@gmail.com>.
Jan,
Just wanted to follow up with on this. If you're looking for tight
integration of wicket with hibernate annotations, please check out the
wicketstuff-hibernate project. I really think this would work well for
you. You've obviously been working in this area for a while, so if there
are any things you can recommend for the wicketstuff-hibernate project that
would help your project, let me know.
http://jroller.com/page/wireframe/?anchor=hibernateannotationcomponentconfigurator
Ryan
On 7/3/07, Jan Vermeulen <ja...@isencia.com> wrote:
>
>
> Thanks for adding the 'INullAcceptingValidator'. This solves our problem
> in a
> clean way.
>
> Just a small suggestion: I've seen that the CompoundValidator does not
> implement this interface. Would it not be correct to assure that the
> CompoundValidator is called always, and that the same logic of checking
> the
> implementation of the interface is applied to all the validators in the
> chain ?
>
> Jan.
> --
> View this message in context:
> http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11410534
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>
Re: Problem with latest changes in FormComponent:validate()
Posted by Eelco Hillenius <ee...@gmail.com>.
> Thanks for adding the 'INullAcceptingValidator'. This solves our problem in a
> clean way.
>
> Just a small suggestion: I've seen that the CompoundValidator does not
> implement this interface. Would it not be correct to assure that the
> CompoundValidator is called always, and that the same logic of checking the
> implementation of the interface is applied to all the validators in the
> chain ?
Sounds reasonable, but I didn't implement it. Matej/ Igor?
Eelco
Re: Problem with latest changes in FormComponent:validate()
Posted by Jan Vermeulen <ja...@isencia.com>.
Thanks for adding the 'INullAcceptingValidator'. This solves our problem in a
clean way.
Just a small suggestion: I've seen that the CompoundValidator does not
implement this interface. Would it not be correct to assure that the
CompoundValidator is called always, and that the same logic of checking the
implementation of the interface is applied to all the validators in the
chain ?
Jan.
--
View this message in context: http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11410534
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: Problem with latest changes in FormComponent:validate()
Posted by Igor Vaynberg <ig...@gmail.com>.
and that all makes perfect sense, however at its current state the
requried/null checks are considered syntactic checks. if not then every
ivalidator out there must first perform the null check, which is annoying.
how about:
class ConstraintValidator extends ... implments IValidatorAddListener {
...
onAdded(Component c) {
FormComponent fc=(FormComponent)c;
fc.setRequired(isrequired());
}
}
-igor
On 6/20/07, Jan Vermeulen <ja...@isencia.com> wrote:
>
>
> Let me try to explain:
>
> First, we have a way of gathering 'constraints' for bean properties, that
> are based either on validator annotations on the bean, or on a settings
> environment (the beanClass & propertyPath are derived from the model of
> the
> formComponent). The endresult is a list of IConstraint's:
>
> List<IConstraint> beanConstraints = EntityUtils.getConstraints(
> ModelUtils.findBeanClass(this), ModelUtils.getPropertyPath
> (this));
>
> On initialization of the formComponent, we gather these constraints, and
> if
> any are available, we add a ConstraintValidator to the component for each
> constraint:
>
> for (IConstraint constraint : beanConstraints) {
> validators.add(new ConstraintValidator(this,constraint));
> }
>
> So after initalization, the formComponent contains a list of validator
> based
> on constraints defined on the property on which it works.
>
> The rest is done by Wicket, when calling the validators in
> formComponent.validate().
>
> Jan.
> --
> View this message in context:
> http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11209048
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>
Re: Problem with latest changes in FormComponent:validate()
Posted by Jan Vermeulen <ja...@isencia.com>.
Let me try to explain:
First, we have a way of gathering 'constraints' for bean properties, that
are based either on validator annotations on the bean, or on a settings
environment (the beanClass & propertyPath are derived from the model of the
formComponent). The endresult is a list of IConstraint's:
List<IConstraint> beanConstraints = EntityUtils.getConstraints(
ModelUtils.findBeanClass(this), ModelUtils.getPropertyPath(this));
On initialization of the formComponent, we gather these constraints, and if
any are available, we add a ConstraintValidator to the component for each
constraint:
for (IConstraint constraint : beanConstraints) {
validators.add(new ConstraintValidator(this,constraint));
}
So after initalization, the formComponent contains a list of validator based
on constraints defined on the property on which it works.
The rest is done by Wicket, when calling the validators in
formComponent.validate().
Jan.
--
View this message in context: http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11209048
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: Problem with latest changes in FormComponent:validate()
Posted by Eelco Hillenius <ee...@gmail.com>.
On 6/19/07, Eelco Hillenius <ee...@gmail.com> wrote:
> Maybe a bit OT, but how exactly do you implement this Jan?
>
> The project I'm working on has:
>
> public final class ValidationListener implements
> IComponentOnBeforeRenderListener {
>
> public void onBeforeRender(Component component) {
> if (component instanceof FormComponent && !component.hasBeenRendered()) {
> processComponent((FormComponent) component);
> }
> }
> ...
>
> and then somewhere in processComponent:
>
> ...
> Column column = field.getAnnotation(Column.class);
> Class<?> type = field.getType();
> ..
> if (column.nullable() == false && type != Boolean.class && type !=
> Boolean.TYPE) {
> component.setRequired(true);
> }
processComponent starts like this:
public static void processComponent(FormComponent component) {
IModel model = component.getModel();
if (model instanceof IPropertyReflectionAwareModel) {
Field field = ((IPropertyReflectionAwareModel) model).getPropertyField();
if (field != null) {
processComponentField(field, (FormComponent) component);
Eelco
Re: Problem with latest changes in FormComponent:validate()
Posted by Eelco Hillenius <ee...@gmail.com>.
Maybe a bit OT, but how exactly do you implement this Jan?
The project I'm working on has:
public final class ValidationListener implements
IComponentOnBeforeRenderListener {
public void onBeforeRender(Component component) {
if (component instanceof FormComponent && !component.hasBeenRendered()) {
processComponent((FormComponent) component);
}
}
...
and then somewhere in processComponent:
...
Column column = field.getAnnotation(Column.class);
Class<?> type = field.getType();
..
if (column.nullable() == false && type != Boolean.class && type !=
Boolean.TYPE) {
component.setRequired(true);
}
Do you something similar, or do you follow an intirely different
aproach (probably, coming from 1.2)?
Eelco
On 6/19/07, Jan Vermeulen <ja...@isencia.com> wrote:
>
> Problem with all these solutions, is that we have to differentiate between
> 'RequiredValidators' and other validators, which means somehow we have to
> know which ones to call when. Because now, there is just a list of
> validators, added to the component, and they get called by Wicket. What I'm
> trying to say is that checking if a field is empty or not can be just a
> validator task. Now, we can no longer simply plug that in as just another
> validator.
>
> I understand that it is handy not to have to check if a value is null, but
> we're paying a price for this that might be higher than the gain ?
>
> Jan.
>
>
> igor.vaynberg wrote:
> >
> > or instead,
> >
> > maybe make that hibernatevalidator also be a behavior, and simply toggle
> > component's setrequired in onattach
> >
> > -igor
> >
> >
> > On 6/19/07, Igor Vaynberg <ig...@gmail.com> wrote:
> >>
> >> i dont know about that. isvalid's contract is that it only returns false
> >> if there are error messages reported against the component.
> >>
> >> i think this would be simple enough
> >>
> >> class hibernatetextfield extends textfield {
> >> valid() {
> >> if (hibernaterequired()) {
> >> if (Strings.isEmpty(getInput()[0])) {
> >> error(new ValidationError().addMessageKey("Required"));
> >> return;
> >> }
> >> }
> >> super.validate();
> >> }
> >>
> >> -igor
> >
>
> --
> View this message in context: http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11200959
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>
Re: Problem with latest changes in FormComponent:validate()
Posted by Jan Vermeulen <ja...@isencia.com>.
Problem with all these solutions, is that we have to differentiate between
'RequiredValidators' and other validators, which means somehow we have to
know which ones to call when. Because now, there is just a list of
validators, added to the component, and they get called by Wicket. What I'm
trying to say is that checking if a field is empty or not can be just a
validator task. Now, we can no longer simply plug that in as just another
validator.
I understand that it is handy not to have to check if a value is null, but
we're paying a price for this that might be higher than the gain ?
Jan.
igor.vaynberg wrote:
>
> or instead,
>
> maybe make that hibernatevalidator also be a behavior, and simply toggle
> component's setrequired in onattach
>
> -igor
>
>
> On 6/19/07, Igor Vaynberg <ig...@gmail.com> wrote:
>>
>> i dont know about that. isvalid's contract is that it only returns false
>> if there are error messages reported against the component.
>>
>> i think this would be simple enough
>>
>> class hibernatetextfield extends textfield {
>> valid() {
>> if (hibernaterequired()) {
>> if (Strings.isEmpty(getInput()[0])) {
>> error(new ValidationError().addMessageKey("Required"));
>> return;
>> }
>> }
>> super.validate();
>> }
>>
>> -igor
>
--
View this message in context: http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11200959
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: Problem with latest changes in FormComponent:validate()
Posted by Igor Vaynberg <ig...@gmail.com>.
or instead,
maybe make that hibernatevalidator also be a behavior, and simply toggle
component's setrequired in onattach
-igor
On 6/19/07, Igor Vaynberg <ig...@gmail.com> wrote:
>
> i dont know about that. isvalid's contract is that it only returns false
> if there are error messages reported against the component.
>
> i think this would be simple enough
>
> class hibernatetextfield extends textfield {
> valid() {
> if (hibernaterequired()) {
> if (Strings.isEmpty(getInput()[0])) {
> error(new ValidationError().addMessageKey("Required"));
> return;
> }
> }
> super.validate();
> }
>
> -igor
>
>
>
> On 6/19/07, Jan Vermeulen <jan.vermeulen@isencia.com > wrote:
> >
> >
> >
> > igor.vaynberg wrote:
> > >
> > > i would not be against making formcomponent.validate() non-final
> > >
> > > -igor
> > >
> >
> > I was thinking of making 'isValid()' non-final, and adding that logic in
> > there.
> >
> > Jan.
> > --
> > View this message in context:
> > http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11200420
> > Sent from the Wicket - Dev mailing list archive at Nabble.com.
> >
> >
>
Re: Problem with latest changes in FormComponent:validate()
Posted by Igor Vaynberg <ig...@gmail.com>.
i dont know about that. isvalid's contract is that it only returns false if
there are error messages reported against the component.
i think this would be simple enough
class hibernatetextfield extends textfield {
valid() {
if (hibernaterequired()) {
if (Strings.isEmpty(getInput()[0])) {
error(new ValidationError().addMessageKey("Required"));
return;
}
}
super.validate();
}
-igor
On 6/19/07, Jan Vermeulen <ja...@isencia.com> wrote:
>
>
>
> igor.vaynberg wrote:
> >
> > i would not be against making formcomponent.validate() non-final
> >
> > -igor
> >
>
> I was thinking of making 'isValid()' non-final, and adding that logic in
> there.
>
> Jan.
> --
> View this message in context:
> http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11200420
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>
Re: Problem with latest changes in FormComponent:validate()
Posted by Jan Vermeulen <ja...@isencia.com>.
igor.vaynberg wrote:
>
> i would not be against making formcomponent.validate() non-final
>
> -igor
>
I was thinking of making 'isValid()' non-final, and adding that logic in
there.
Jan.
--
View this message in context: http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11200420
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: Problem with latest changes in FormComponent:validate()
Posted by Igor Vaynberg <ig...@gmail.com>.
i would not be against making formcomponent.validate() non-final
-igor
On 6/19/07, Jan Vermeulen <ja...@isencia.com> wrote:
>
>
> That means you can never put 'required' logic in a validator. And that's
> just
> what we want to do, because that's the way we can integrate with the
> Hibernate validators. I saw some time ago on the forum that there was
> interest in integrating the Wicket validators with the Hibernate validator
> framework: well, we are doing this, and @NotEmpty is one of these
> Hibernate
> validation annotations.
>
> Could it at least be something we can override ?
>
> Jan.
> --
> View this message in context:
> http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11199762
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>
Re: Problem with latest changes in FormComponent:validate()
Posted by Jan Vermeulen <ja...@isencia.com>.
That means you can never put 'required' logic in a validator. And that's just
what we want to do, because that's the way we can integrate with the
Hibernate validators. I saw some time ago on the forum that there was
interest in integrating the Wicket validators with the Hibernate validator
framework: well, we are doing this, and @NotEmpty is one of these Hibernate
validation annotations.
Could it at least be something we can override ?
Jan.
--
View this message in context: http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11199762
Sent from the Wicket - Dev mailing list archive at Nabble.com.
Re: Problem with latest changes in FormComponent:validate()
Posted by Igor Vaynberg <ig...@gmail.com>.
the added code is there to guard validators against having to check for
null. if we remove it then the first thing all validators should do is check
if validatable.getvalue() is null - which gets annoying and causes NPEs if
you forget.
wicket has a facility for setting whether or not the component is required,
so that should be used.
-igor
On 6/19/07, Jan Vermeulen <ja...@isencia.com> wrote:
>
>
> The code below is the actual code of FormComponent:validate()
>
> public final void validate()
> {
> validateRequired();
> if (isValid())
> {
> convert();
>
> if (isValid() && getConvertedInput() != null)
> {
> validateValidators();
> }
> }
> }
>
> The code in bold has been added. Problem is that we are using a validator
> (that ends up consulting a Hibernate validator annotation) to determine if
> a
> field is required for an entity.
>
> But with the added code, this validator never gets called ! If we already
> passed through an isValid(), could you not suppose that the extra check is
> not needed ? Or should it not be part of the isValid() logic, in a way
> that
> is overridable (isValid() is final) ?
>
> Jan.
> --
> View this message in context:
> http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11193140
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>
Re: Problem with latest changes in FormComponent:validate()
Posted by Maurice Marrink <ma...@gmail.com>.
more logical would imo be if the validators could be chained and then
let them deside if they want to do And or Or logic. But i have never
worked with validators before so i am not sure if this already
possible or not.
my 2c
Maurice
On 6/19/07, Jan Vermeulen <ja...@isencia.com> wrote:
>
> The code below is the actual code of FormComponent:validate()
>
> public final void validate()
> {
> validateRequired();
> if (isValid())
> {
> convert();
>
> if (isValid() && getConvertedInput() != null)
> {
> validateValidators();
> }
> }
> }
>
> The code in bold has been added. Problem is that we are using a validator
> (that ends up consulting a Hibernate validator annotation) to determine if a
> field is required for an entity.
>
> But with the added code, this validator never gets called ! If we already
> passed through an isValid(), could you not suppose that the extra check is
> not needed ? Or should it not be part of the isValid() logic, in a way that
> is overridable (isValid() is final) ?
>
> Jan.
> --
> View this message in context: http://www.nabble.com/Problem-with-latest-changes-in-FormComponent%3Avalidate%28%29-tf3945885.html#a11193140
> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>
>