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

validateBean - validation message per bean's property #317

Closed
ptrwis opened this Issue Sep 24, 2016 · 7 comments

Comments

Projects
None yet
2 participants
@ptrwis

ptrwis commented Sep 24, 2016

Hello.
I have custom, class-level constraint validator (implementing javax.validation.ConstraintValidator) for a "Car" entity. This Car has a property called "color". The validator leaves a message per this property, like this:

context.buildConstraintViolationWithTemplate( message )
    .addPropertyNode( "color" ) //Car.color
    .addConstraintViolation();

Now, on my page, I have a form for creating a new Car, with selectOneMenu for choosing a "color":

<h:form>
    <h:outputText value="color" />
    <h:selectOneMenu value="#{createCar.car.color}" id="colorSelect" >
        <f:converter binding="#{viewScopedFacesConverter}" />
        <f:selectItems value="#{createCar.allColors}" var="color"   />
    </h:selectOneMenu>
    <h:message for="colorSelect" />
    <h:commandButton value="create" action="#{createCar.on_createCar_ButtonClick}" />
    <o:validateBean value="#{createCar.car}"  />
</h:form>
<h:messages  />

(other fields omitted for brevity, createCar is a controller)
As you can see, I'm using omnifaces/validateBean component to trigger my custom validator.
Everything works (almost) fine. The problem is that validateBean component leaves the message in <h:messages> (I believe in fact it leaves a message with form being the client), not in <h:message for="colorSelect" />, what I would expect.
Please consider adding support for leaving a message per property when validating whole bean.

Here is full source code of the validator:

public class CarModelVsColorValidatorImpl implements ConstraintValidator<CarModelVsColorValidator, Car>
{

    @Override
    public boolean isValid(Car car, ConstraintValidatorContext context) {
    if( car==null )
        return true;
        boolean isValid = true; 

        String message="";

        // (...)
        if ( car.getModel().equals("BMW") && car.getColor().equals("BLUE") )
        {
            isValid = false;
            message = "Bmw can't be blue!";
        }

        if( isValid == false ){
            context.disableDefaultConstraintViolation();
            context.buildConstraintViolationWithTemplate( message )
                   .addPropertyNode( "color" )
                   .addConstraintViolation();
        }

        return isValid;
    }

    @Override
    public void initialize(CarModelVsColorValidator constraintAnnotation) {

    }

}
@BalusC

This comment has been minimized.

Show comment
Hide comment
@BalusC

BalusC Sep 25, 2016

Member

I have added showMessageFor attribute with almost same logic as existing ValidateMultipleFields components (<o:validateAll>, <o:validateEqual>, etc).

<o:validateBean ... showMessageFor="colorSelect" />

Supported values are @form (default), @all (all associated inputs), @global (for globalOnly="true") and any space separated string of (relative) client IDs. Additionally, I have added logic which does input.setValid(false), which seems to be overlooked in the implementation. This way styling such as <o:highlight> should also work on this.

Member

BalusC commented Sep 25, 2016

I have added showMessageFor attribute with almost same logic as existing ValidateMultipleFields components (<o:validateAll>, <o:validateEqual>, etc).

<o:validateBean ... showMessageFor="colorSelect" />

Supported values are @form (default), @all (all associated inputs), @global (for globalOnly="true") and any space separated string of (relative) client IDs. Additionally, I have added logic which does input.setValid(false), which seems to be overlooked in the implementation. This way styling such as <o:highlight> should also work on this.

@BalusC

This comment has been minimized.

Show comment
Hide comment
@BalusC

BalusC Sep 25, 2016

Member

It's available in today's latest 2.6-SNAPSHOT. Let me know if that does the job for you.

Member

BalusC commented Sep 25, 2016

It's available in today's latest 2.6-SNAPSHOT. Let me know if that does the job for you.

@ptrwis

This comment has been minimized.

Show comment
Hide comment
@ptrwis

ptrwis Sep 26, 2016

Well... no it's not working. I'm trying latest 2.6-20160926.134838-12. The message is still showing under <h:messages>. But I'm not sure if showMessageFor attribute is a good idea- what if ConstraintValidator throw many ConstraintViolations with different PropertyNodes, or there are many class-level ConstraintValidators on one entity?

ptrwis commented Sep 26, 2016

Well... no it's not working. I'm trying latest 2.6-20160926.134838-12. The message is still showing under <h:messages>. But I'm not sure if showMessageFor attribute is a good idea- what if ConstraintValidator throw many ConstraintViolations with different PropertyNodes, or there are many class-level ConstraintValidators on one entity?

@BalusC

This comment has been minimized.

Show comment
Hide comment
@BalusC

BalusC Sep 26, 2016

Member

Well... no it's not working. I'm trying latest 2.6-20160926.134838-12. The message is still showing under <h:messages>.

This statement is ambiguous. Does it show up in <h:message for="colorSelect" /> or not? The <h:messages> will by default display all messages, also the already displayed ones (which you can turn off with redisplay="false" or globalOnly="true"). At least, it works for me and it's also covered in IT as shown here: 99e5803

But I'm not sure if showMessageFor attribute is a good idea- what if ConstraintValidator throw many ConstraintViolations, or there are many class-level ConstraintValidators on one entity?

You can also use a <h:messages for="colorSelect">.

After re-reading your question I guess you'd like to associate the message with the input component bound to the property as specified by addPropertyNode(). But the ConstraintViolation has only a getPropertyPath() method which doesn't return expected values.

Member

BalusC commented Sep 26, 2016

Well... no it's not working. I'm trying latest 2.6-20160926.134838-12. The message is still showing under <h:messages>.

This statement is ambiguous. Does it show up in <h:message for="colorSelect" /> or not? The <h:messages> will by default display all messages, also the already displayed ones (which you can turn off with redisplay="false" or globalOnly="true"). At least, it works for me and it's also covered in IT as shown here: 99e5803

But I'm not sure if showMessageFor attribute is a good idea- what if ConstraintValidator throw many ConstraintViolations, or there are many class-level ConstraintValidators on one entity?

You can also use a <h:messages for="colorSelect">.

After re-reading your question I guess you'd like to associate the message with the input component bound to the property as specified by addPropertyNode(). But the ConstraintViolation has only a getPropertyPath() method which doesn't return expected values.

@ptrwis

This comment has been minimized.

Show comment
Hide comment
@ptrwis

ptrwis Sep 29, 2016

After re-reading your question I guess you'd like to associate the message with the input component bound to the property as specified by addPropertyNode(). But the ConstraintViolation has only a getPropertyPath() method which doesn't return expected values.

Yes this is what I wanted to do. ConstraintViolation has also methods like getLeafBean or getRootBean. They reference specific bean's instance, the same as ValueExpression.getValue() does. I thought that maybe somehow combining getRootBean with getPropertyPath and comparing with properties returned by ExpressionLanguage (ValueExpression etc) it could be possible to do what I was asking for. Now I think that maybe there should be another phase in JSF like "validate models", because it's obvious (?) that this (second) validation should be done after "update model values" phase.

ptrwis commented Sep 29, 2016

After re-reading your question I guess you'd like to associate the message with the input component bound to the property as specified by addPropertyNode(). But the ConstraintViolation has only a getPropertyPath() method which doesn't return expected values.

Yes this is what I wanted to do. ConstraintViolation has also methods like getLeafBean or getRootBean. They reference specific bean's instance, the same as ValueExpression.getValue() does. I thought that maybe somehow combining getRootBean with getPropertyPath and comparing with properties returned by ExpressionLanguage (ValueExpression etc) it could be possible to do what I was asking for. Now I think that maybe there should be another phase in JSF like "validate models", because it's obvious (?) that this (second) validation should be done after "update model values" phase.

@ptrwis

This comment has been minimized.

Show comment
Hide comment
@ptrwis

ptrwis Oct 10, 2016

I was trying to solve it by myself but I'm not sure if this is doable. For now I'll close the issue. Thank you for your help anyway :)

ptrwis commented Oct 10, 2016

I was trying to solve it by myself but I'm not sure if this is doable. For now I'll close the issue. Thank you for your help anyway :)

@ptrwis ptrwis closed this Oct 10, 2016

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 21, 2017

FWIW I implemented the requested feature in #343.

After re-reading your question I guess you'd like to associate the message with the input component bound to the property as specified by addPropertyNode(). But the ConstraintViolation has only a getPropertyPath() method which doesn't return expected values.

Quite true, my implementation naïvely expects Path#toString() to return something like 'color' (which it does with Hibernate), and doesn't make any effort to navigate more complex/nested Path instances. However the fall-through of treating unidentified ConstraintViolation→UIComponent mappings as if "@form" were specified works well enough.

ghost commented Jan 21, 2017

FWIW I implemented the requested feature in #343.

After re-reading your question I guess you'd like to associate the message with the input component bound to the property as specified by addPropertyNode(). But the ConstraintViolation has only a getPropertyPath() method which doesn't return expected values.

Quite true, my implementation naïvely expects Path#toString() to return something like 'color' (which it does with Hibernate), and doesn't make any effort to navigate more complex/nested Path instances. However the fall-through of treating unidentified ConstraintViolation→UIComponent mappings as if "@form" were specified works well enough.

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