Bean Validation integration

[follis]

Originally opened as bug 7291 by rmannibucau

--------------Original Comment History---------------------------- Comment from = rmannibucau on 2015-09-05 09:34:09 +0000

As optional it would be nice to be able to validate parameters of all components using bean validation.

here few examples to make it concrete:

• chunk processor: Object process(@Valid MyItem item);
• chunk writer: void writeItems(@Valid List xs)
• etc...

Bean Validation 1.1 has the API to validate input parameters and returned values.

Side note: for CDI case it is supported out of the box if JBatch components are CDI beans and not JBatch beans supporting CDI injections - this can be a point to enhance/clarify maybe.

---

Comment from = ScottKurz on 2015-10-06 21:26:18 +0000

Not sure if you already had covered this, but just to be clear: it would be nice to validate the injected @BatchProperty values.

My (limited) understanding is that, even when the batch artifact is loaded as a CDI bean and the property injected via CDI, that the validation won't be done automatically already.

...

Like you said, it would be helpful to clarify whether we're defining this in the CDI integration space or the DI-neutral space.

[scottkurz]

Seems #43 has some overlap here:

[rmannibucau]

BatchProperty validation can be done with cdi (2.0) since @AroundConstruct is now supported and integrated in bean validation so don't think anything is needed to consider this issue solved?

[scottkurz]

We'd at least want to add TCK tests

[rmannibucau]

@scottkurz maybe I'm wrong - and I certainly didn't follow the process closely enough to be sure - but isn't it of either bean validation (for cdi integration) scope or jakartaee (umbrella) TCK responsability more than jbatch one to do such overlapping TCK? JBatch by itself does not have to require anything since it is an IoC/Bean Validation only integration so it is already covered by bean validation tck itself and JBatch inherits from it, no? To be concrete, JBatch does not test that CDI scopes work in JBatch for ex but assumes it.

[mminella]

Forgive my ignorance, but are there other Jakarta specs that add Bean Validation through out like this request?

[rmannibucau]

JAXRS, EJB etc do and seems to have their own TCK (https://github.com/eclipse-ee4j/jaxrs-api/issues/939) but it is mainly because it is not CDI focused spec (sadly but historically) so I hope new specs focusing and aware of CDI dont need to redo the work N times (integration/spec/tests) since technically there is really no need since all is already defined in CDI and Bean Validation spec properly.

[scottkurz]

I'm going to just stop and state my ignorance here.

I'm not purposely arguing for an extra layer of integration testing just for batch if this is already well-defined by CDI + Bean Val integration.

If this is only well-defined when CDI is used then I'm NOT proposing we separately define the integration for non-CDI.

Again, we had the case of validating input parms passed to artifacts within the chunk loop that Romain originally raised.

And I also added validation like this:

    @Inject @BatchProperty
    @Min(value = 18, message = "Age should not be less than 18")
    @Max(value = 150, message = "Age should not be greater than 150")
    private int age;

When I originally looked into this awhile back, this was not automatically validated with bean validation even when the batch artifact was loaded as a CDI bean (at least in jbatch), which is why I was assuming there was some more work to do here.

[chengfang]

For validation of batch property injection: I tested with JBeret and WildFly, and got the same result as Scott, validation of constraints like @Min @Max etc are not happening. So looks like more work is needed here. And I think it is a good enhancement to have in batch spec next.

For validation of batch artifacts input and output, I think it's less important. For one thing, it will only be useful if all artifacts are refactored to support java generics, so it's clear what the target type the application is trying to validate. Also these input output types are more likely to be complex user types, other than primitives and scalar types, so the value of built-in javax.validation.constraints are not much. It's better for the application to handle its own validation. For example, JBeret does this sort of validation in test classes like: https://github.com/jberet/jberet-support/blob/f3d165a920a3a6667b205f54d7062d400c1c6af8/src/test/java/org/jberet/support/io/StockTradeBase.java#L40

On Tue, Apr 6, 2021 at 11:13 AM Scott Kurz @.***> wrote:

I'm going to just stop and state my ignorance here.

I'm not purposely arguing for an extra layer of integration testing just for batch if this is already well-defined by CDI + Bean Val integration.

If this is only well-defined when CDI is used then I'm NOT proposing we separately define the integration for non-CDI.

Again, we had the case of validating input parms passed to artifacts within the chunk loop that Romain originally raised.

And I also added validation like this:

@Inject @BatchProperty
@Min(value = 18, message = "Age should not be less than 18")
@Max(value = 150, message = "Age should not be greater than 150")
private int age;

When I originally looked into this awhile back, this was not automatically validated with bean validation even when the batch artifact was loaded as a CDI bean (at least in jbatch), which is why I was assuming there was some more work to do here.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/eclipse-ee4j/batch-api/issues/82#issuecomment-814201306, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAP3UEYDSFXUNVBSYPL5ZD3THMQI7ANCNFSM4O2U5PMA .

[rmannibucau]

@chengfang @scottkurz what about coding it as expected by bean validation:

@Dependent public class MyComponent ... {
@Inject MyComponent(@BatchProperty int age) {}
}

?

Agree for all other cases it is saner to let the application do the validation itself. Once reason auto-validation is not great - including in bean validation spec - is the lack of group and full API support

@Dependent public class MyComponent ... {
 // injections

 @Inject Validator validator;

@PostConstruct void validate() { throwIfNotEmpty(validator.validate(this, Mygroup.class)); }
}

does not look crazy to do when used - bean validation is less and less used in apps, in particular since j8 introduced some validation methods (requireNonNull and friends) since it implies some bigger stack which are rarely justified and require a heavy investment to be useful with relevant error message (often business related).

So overall what about:

1. well documenting the CDI + bean validation usage (maybe as an example)
2. maybe show the postconstruct example if you think it is relevant
3. not go further

[scottkurz]

Seems we're not going to get to this in 2.1.