Fix #5752 Weld injector reinitialization

[BrentDouglas]

Initialize weld once when the injector is initialized rather than per lookup. Also changes the injector to use BeanManager#resolve rather than cycling through matching beans.

[scottkurz]

Brent,

Is there a test you could point me towards to validate this (not the original issue, just simply that once it's merged, Weld will work)? Thanks

[BrentDouglas]

I'll add one now.

Brent On Nov 8, 2014 8:20 PM, "Scott Kurz" notifications@github.com wrote:

Brent,

Is there a test you could point me towards to validate this (not the original issue, just simply that once it's merged, Weld will work)? Thanks

— Reply to this email directly or view it on GitHub https://github.com/WASdev/standards.jsr352.jbatch/pull/4#issuecomment-62269626 .

[BrentDouglas]

I've added a test for weld. In the process also fixed the @BatchProperty producer and discovered that this implementations limits producer methods to return their implementation class. e.g. This will not be loaded:

@Produces
@Named("theBatchlet")
public Batchlet produce(final @New TheBatchlet that) {
    return that;
}

While this will:

@Produces
@Named("theBatchlet")
public TheBatchlet produce(final @New TheBatchlet that) {
    return that;
}

Fixing it will require either munging the required interface out of the injected bean's class or alternatively changing IBatchArtifactFactory#load(String) to IBatchArtifactFactory#load(String, Class) where the interface can be passed in. It is outside the scope of this PR.

[scottkurz]

Just wanted to note I was planning to consider this after we finished 1.0.1 (still integrating w/ GF).

[BrentDouglas]

This wont affect GlassFish at all. GF would be using CDIBatchArtifactFactoryImpl rather than WeldSEBatchArtifactFactoryImpl. This PR is broken at the moment though as something related to setting the services has changed and WeldTest is not using the WeldSE... implementation. Is there a recommended way of setting the services to use?

[scottkurz]

It seems the impact to GF is only in the BatchProducerBean which is also used on the CDIBatchArtifactFactoryImpl path. And actually the change is largely insignificant except for the removal of the null guard. I guess I could merge it in and leave that null guard.

As far as the setting of the services I have that fixed. I'll commit shortly.

[BrentDouglas]

Ok, afaict there is no way to set the service to use to instantiate a single job operator. This now just sets the artifact factory to the weld one as a system property so all the tests will be using it. I had not realized before that all the config was statically cached and probably only checked it worked with -Dtest=WeldTest rather than a full suite run.

[BrentDouglas]

Oh, OK never mind, ignore that last comment.

[scottkurz]

The bigger dilemma is that the choice of service impls is read once and hardened, so a suite of tests requiring different selections can't simply run all in one JVM.

Maybe this is an unusual approach but the way I've been dealing with this is in the com.ibm.jbatch.container module is to use the failsafe integration tests to fork one JVM per test class.... while letting the surefire tests run all in one JVM.

So it's not so much about the general concept of unit vs integration test here as it is the JVM lifecycle. Haven't tested extensively if this is the best approach..it just seems the unit tests already take a long time to run..probably in part because of all the sleeping in them.

Yes, I'll commit shortly... just commenting since I saw you working on this.

[scottkurz]

OK merged almost all your change but I left the null guard in BatchProducerBean. Closing this PR.

[scottkurz]

By the way..thanks for this one too !