[Feature] jsr 303 bean validation for class level constraints #33

Closed
djmj opened this Issue May 21, 2014 · 20 comments

Projects

None yet

4 participants

@djmj
djmj commented May 21, 2014

What about a jsf tag that triggers class level constraints on a given bean within scope of a jsf form?

Having some class level constraints validation must manually be called within bean.

<o:validateBean value="#{bean.foo}"/>

This way "validateBean" would mean what it says and not just validate a single property.

regards mj

@ova2
ova2 commented Jun 27, 2014

I've just started to implement a GraphValidator like in RichFaces. https://access.redhat.com/site/documentation/en-US/Red_Hat_JBoss_Web_Framework_Kit/2/html/RichFaces_Component_Reference_Guide/sect-Component_Reference-Validation-richgraphValidator.html
This will allow cross field validation.

I think this validator is a good candidate for OmniFaces. Would the OmniFaces team accept my pull request when the validator is ready to use?

Thanks. Oleg.

@BalusC
Member
BalusC commented Jun 27, 2014

If I understand you correctly, you're looking for the same functionality as available by <o:validateBean validationGroups>. See also http://showcase.omnifaces.org/validators/validateBean

@ova2
ova2 commented Jun 30, 2014

Where is the class level validation in the o:validateBean?

@BalusC
Member
BalusC commented Jun 30, 2014

It's the default already when using JSR303 bean validation, right? The o:validateBean just enables more finer grained control over that.

Or perhaps I misunderstood both of you. Having a real world use case would probably help in a better understanding.

@ova2
ova2 commented Jun 30, 2014

JSF can not validate class or method level constraints. Only constraints on fields. What I need in my project:

Execution of a method annotated with @AssertTrue in the JSF Validation Phase (not in the Invoke Application Phase).

That means, we need a similar construct as in the mentioned above GraphValidator. It clones the bound bean while PostValidate, set local values into the cloned bean (or what ever is set in this phase) and invokes validate() on the bean via JSR303 API.

Probably we have also to consider first available constraint annotation like PrimeFaces does it (cann't remember the internal class name in PrimeFaces).

@djmj
djmj commented Jul 1, 2014

<f:validateBean> and <o:validateBean> is not validating a "bean", it only validates a single property that is bound to an UIInput.

A simple example with custom validators would be:

/* validate if barcode is valid for given type*/
@ValidateProduct(groups = {BarcodeType.class, Default.class})
public class Product
{
    private String barcode;
    private BarcodeType barcodeType;
    @Valid
    private ProductDim dimension;
}

/* or use new @AssertTrue or other extended validation annotations */
@ValidProductDim 
public class ProductDim
{
    private int width;
    private int height;
    private int depth;
}

JSF:
Imagine a wizard with following step:

<h:form>
    <p:tab id="tabBarcode">
        <p:messages/>
        <p:inputText value="#{bean.product.barcode}"/>
        <p:selectOneMenu value="#{bean.product.barcodeType}"/>
        <!-- on submit or tab-change lets validate the combination -->
        <o:validateBean value="#{bean.product} groups="BarcodeType"/>
    </p:tab>
    <!-- other tabs -->
</h:form>

Currently this must be handled in backing bean after each step or complete form submit:

/* Primefaces tab flow eventhandler */
public String onFlowProcess(final FlowEvent event)
{
    final Validator validator = Validation.buildDefaultValidatorFactory().getValidator();

    switch (event.getOldStep())
    {
        case "tabBarcode":

        final Set<ConstraintViolation<Object>> cvs = validator.validate(object, BarcodeGroup.class);
        if (!cvs.isEmpty())
            // add error message or throw new ConstraintViolationException(cvs);
    }
}

This is much code to write some something thats supposed to be a java-ee standard.

This would be very helpfull.

@ova2
ova2 commented Jul 2, 2014

Agree. And this is not only much code. This is the wrong JSF phase too. The listener onFlowProcess is fired much later than validation phase. Wrong values are already set in bean!

@arjantijms
Member

As it appears, a spec issue was logged for this over half a decade ago: https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-543

@arjantijms arjantijms pushed a commit that referenced this issue Oct 21, 2014
arjan.tijms #33 Initial attempt at validating at class level. WIP! dcb40e1
@arjantijms arjantijms pushed a commit that referenced this issue Oct 22, 2014
arjan.tijms #33 Fix npe 5a0c9ea
@arjantijms arjantijms pushed a commit that referenced this issue Oct 22, 2014
arjan.tijms #33 Basic implementation of validating beans at the class level with a
given group.
def5026
@arjantijms
Member

Guys, I did a basic implementation of the class level constraint validation as per the commits shown above. There's a demo at omnifaces/showcase@ff430bd#diff-1

This now works much like djmj's example. What it does not (yet) do is what Oleg talked about; cloning of the bean, simulating update model on the cloned bean during the VALIDATION phase, and then doing the class level validations.

@arjantijms
Member

A possible algorithm for the simulating of update model mentioned above:

  • Check all components in parent form of o:validateBean
    • component == editableValueHolder?
      • get target base/property of value expression (e.g. #{validateClassLevelBean.product.number1} -> product/number1)
      • base == o:validateBean's value? (E.g. #{validateClassLevelBean.product}" == product)
        • Add extra "validator" -> just captures value for that property (e.g. "number1" = [value])
  • At end of validate phase:
    • Clone base
    • Set base's properties based on captured values
    • Do bean validation against base

I was thinking of doing this as an optional behavior.

So default is then the basic behavior as is happening now: validation against the actual instance in the post update model phase. Disadvantage: not really JSF like, model is updated with invalid values and the user has to check if there was a validation error. Advantage: Simple, lightweight, and no cloning of the model object needed.

The cloning option has basically the opposite advantages/disadvantages.

To do the cloning/copying a couple of strategies could be used:

  • Cloning via the Cloneable interface
  • The old Serialization/deserialization trick
  • A copy constructor of the bean
  • A user provided type that knows how to copy the bean

Examples of the last one in action:

<o:validateBean value="#{validateClassLevelBean.product}" method="validateCopy" copier="com.me.MyBeanCopier" />

And/or:

<o:validateBean value="#{validateClassLevelBean.product}" method="validateCopy" copier="#{someBean.someCopier}" />

The Copier itself may just be an implementation of a simple interface like:

public interface Copier {
    Object copy(Object in);
}
@ova2
ova2 commented Oct 22, 2014

I would clone the base object to validate in the proper JSF phase and don't pollute model with invalid values. All what I need is a working @AsserTrue in JSF.

I don't know if we really need a copier. Checking Cloneable interface and old Serialization/deserialization trick (as fallback) would be enough in my opinion.

@arjantijms
Member

Thanks for the feedback Oleg :)

I was wondering about the copier, but the reason I thought it might be handy is to give the user some control over the process.

For instance, the bean might not be cloneable or serializable, and could come from another package (module) that the UI people do not directly have access to. If the bean is a model object this may not be entirely unthinkable.

Also, the bean could theoretically have some expensive dependencies (maybe some large array) that the UI developer knows does not have to be cloned/copied for the simulation.

Thinking of it, another option may be:

  • new Instance - don't clone/copy the class but create a new instance based on its type

Anyway, it will all be optional of course ;)

@ova2
ova2 commented Oct 22, 2014

Hallo Arjan,

"... and could come from another package (module) that the UI people do not directly have access to" - the copier makes sense then. Thanks.

@arjantijms arjantijms added a commit that referenced this issue Oct 23, 2014
@arjantijms arjantijms #33 Implemented value collecting for each property of the base that's
the target of o:validateBean
c167e7a
@arjantijms arjantijms added a commit that referenced this issue Oct 26, 2014
@arjantijms arjantijms #33 Added copying/cloning of target bean and setting collected values on
it. Also added switches and defaults to set validation mode and a custom
copier.
5fdfaac
@arjantijms
Member

Hi guys,

Per the last commits I implemented the plan as outlined above. There's a demo available at http://snapshot.omnifaces.org/validators/validateBean

One design choice; when validation fails in the post validation phase event listener, the code skips directly to render response, so that the invoke application phase won't be executed. I think that's the only sane option, but let me know what you think.

I'll push a new snapshot to the sonatype repo later today.

@ova2
ova2 commented Oct 27, 2014

"when validation fails in the post validation phase event listener, the code skips directly to render response, so that the invoke application phase won't be executed. I think that's the only sane option, but let me know what you think."

That's correct. Thanks a lot Arjan.

P.S. The model class Product.java is missing in the showcase, the source tab.

@arjantijms
Member

Thanks for the feedback Oleg, appreciate it :)

I've added Product.java in the showcase as well as a few other important classes for the example. I also published the new OmniFaces 2.0-snapshot that contains all these changes.

If there's nothing else to be done and no bugs are found then I think we can finally close this issue.

@ova2
ova2 commented Oct 28, 2014

Good stuff, thanks a lot. I think you can close this issue. I will probably mention this in the PrimeFaces Cookbook, 2. Edition, while writing about validation :-).

@arjantijms
Member

Cool! Looking forward to that :)

All right, I'll close the issue now then. Both you and djmj, thanks a lot again for this great feature suggestion.

@arjantijms arjantijms closed this Oct 28, 2014
@djmj
djmj commented Oct 30, 2014

Thank you all for implementing it! Did not had time to test it yet, but I trust you.
Don't know what I would do without omnifaces :),

@djmj
djmj commented Jan 20, 2017

This bean validation ticket looks related to the cloning / proxy problem.

https://hibernate.atlassian.net/browse/HV-653

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