ManagedExecutorService and ContextService default resources as injectable CDI beans

[njr-11]

Some CDI users would like to be able inject the default instances of ManagedExecutorService, ManagedScheduledExecutorService and/or ContextService into CDI beans with @Inject instead of @Resource.

For example,

@Inject
ManagedExecutorService executor;

instead of

@Resource
ManagedExecutorService executor;

in order to get the default resource "java:comp/DefaultManagedExecutorService".

This can currently be done within the application supplying a CDI producer, but the Jakarta EE product provider could have it built-in and make that unnecessary and some might already be doing so. It would be nice to have standardization of one or the other approach so that applications are portable.

[arjantijms]

I did actually wonder why with the entire platform moving to CDI we actually bothered with @Resource in the first place.

It's probably historical, as Concurrency has it roots tracing back as one of the oldest specs in Jakarta EE (despite having its first release much later). Maybe we can even go a step further though and deprecate @Resource injection for a next version?

[hantsy]

How to process the JDNI name/lookup when using @Inject instead? JNDI is still vital for Jakarta EE?

[njr-11]

How to process the JDNI name/lookup when using @Inject instead? JNDI is still vital for Jakarta EE?

@Inject with no qualifiers would receive the default instance of ManagedExecutorService. If we want to also use @Inject for differently configured instances of ManagedExecutorService, that would require the use of qualifiers, probably pointing at the ManagedExecutorDefinition name attribute, and could have some scoping concerns.

[njr-11]

Here are some examples. First, the straightforward case of the default instances:

@Resource
ContextService contextSvc;

@Resource
ManagedExecutorService executor;

could instead be done as:

@Inject
ContextService contextSvc;

@Inject
ManagedExecutorService executor;

In the case of configured instances, the following is currently possible:

@Resource(lookup = "java:app/MyExecutor")
ManagedExecutorService myExecutor;

@ManagedExecutorDefinition(name = "java:app/MyExecutor", ...)

To do this with @Inject, first we would need to define the name without the java:, just,

@ManagedExecutorDefinition(name = "MyExecutor", ...)

It turns out that names that don't start with java:comp, java:module, java:app, or java:global are actually allowed already, but unfortunately get interpreted as a java:comp/env, so in this case, java:comp/env/MyExecutor. However, we could overload that meaning to also generate a CDI producer with a qualifier having the name.

We would need to standardize the qualifier. Reusing jakarta.inject.Named seems ideal for this, but I'm told not to use it because the CDI spec reserves that qualifier for other purposes. So lacking that maybe we introduce something like an @Identifier qualifier in the Concurrency spec for this,

@Inject
@Identified("MyExecutor")
ManagedExecutorService myExecutor;

It seems like this should be possible then for both the default instances and configured instances of these resources. I'd be curious what thoughts others have on this.

[OndroMih]

I'm definitely for providing a default producer for injecting ManagedExecutorService, ContextService and anything else that can be injected using @Resource now (e.g. ManagedScheduledExecutorService, ManagedThreadFactory), as in the original proposal by @njr-11.

In other cases, I'm not sure that we need a different mechanism than @Resource. In my view, it's always better practice to define logical CDI qualifiers for beans by the user, such as @BackgroundJobs for an executor to run background jobs, or @SequentialExecutor to use a single thread and run tasks sequentially. Rather than inject an executor service or other service by its textual name.

We should rather encourage users to create their own qualifiers and add the mapping to a producer, like this:

@ApplicationScoped
public class ExecutorProducer {

    @Resource(lookup = "java:app/backgroundJobsExecutor")
    @Produces
    @BackgroundJobs
    ManagedExecutorService executor;

    @Resource(lookup = "java:app/sequentialExecutor")
    @Produces
    @SequentialExecutor
    ManagedExecutorService executor;
}

Replacing @Resource with a general qualifier may be useful to make the API better aligned with CDI but this shouldn't be in scope of this issue.

[OndroMih]

We should also remember that some applications may already contain producers for the default concurrency services, such as:

@ApplicationScoped
public class ExecutorProducer {

    @Resource
    @Produces
    ManagedExecutorService executor;

}

So a default producer should be created by the container only if there's no producer defined for it already.

[hantsy]

I would like to simplify the JNDI name as CDI naming directly.

@ManagedExecutorDefinition(name = "MyExecutor", ...)

And inject it directly.

@Inject
@Named("MyExecutor")
ManagedExecutorService executor;

[arjantijms]

See https://github.com/jakartaee/jakartaee-platform/issues/355

There is a wish to root out @Resource entirely from the platform, and IMHO that wish is not wrong.

What about something like:

Define ManagedExecutor:

@MyExecutor
@ManagedExecutorDefinition(
    hungTaskThreshold = 120000,
    maxAsync = 5)
@ApplicationScoped
public class ExecutorProducer {

}

Inject ManagedExecutor:

@Inject
@MyExecutor
ManagedExecutorService executor;

So simply do not use String based names at all. Only use typesafe annotations.

[njr-11]

What about something like:

Define ManagedExecutor:

@MyExecutor
@ManagedExecutorDefinition(
    hungTaskThreshold = 120000,
    maxAsync = 5)
@ApplicationScoped
public class ExecutorProducer {

}

Inject ManagedExecutor:

@Inject
@MyExecutor
ManagedExecutorService executor;

So simply do not use String based names at all. Only use typesafe annotations.

That's an interesting approach. Avoiding String names is appealing. The part I don't understand is how it can be assumed that @MyExecutor will be made into a qualifier for a particular @ManagedExecutorDefinition here though. You can put multiple @ManagedExecutorDefinition onto a class, or a mixture of different resource definition types, or use annotations on a class for other purposes, and so forth, so there is no way of knowing for sure which if any @MyExecutor applies to. I suppose it could be moved inside of the @ManagedExecutorDefinition, like this,

@ManagedExecutorDefinition(
    hungTaskThreshold = 120000,
    maxAsync = 5,
    qualifier = MyExecutor.class)
@ApplicationScoped
public class ExecutorProducer {
}

which would make it precise enough, but comes at a cost of adding some awkwardness.

[OndroMih]

What about using the first approach when there’s only one definition and the second when there are more definitions?

If the first approach is used with multiple definitions, deployment would fail, with a message that suggests using the second approach.

[OndroMih]

However, @Resource would still be needed to refer to executors defined outside of the app. But it’s not worth dropping support for that and replace it with another annotation. @Resource is a general annotation from the Annotations spec, I would simply keep it for the purpose of injecting via JNDI if needed.

If a qualifier is used with @ManagedExecutorDefinition, the name parameter should become optional, with a generated JNDI name if omitted.

[arjantijms]

However, https://github.com/resource would still be needed to refer to executors defined outside of the app.

The question is whether that is needed. We don't need @resource to refer to e.g. an HttpAuthenticationMechanism defined outside the app.

At some point people thought it would be absolutely necessary to define CDI beans outside the app and/or using XML (a sub-project was even started for it), but eventually it became clear this wasn't needed. It just reflected the old way of thinking (e.g. the managed-bean element in faces-config.xml).

[arjantijms]

so there is no way of knowing for sure which if any @MyExecutor applies to

We may need to restrict the cases where qualifiers can be used for ManagedExecutorDefinition (and other definitions).

We may use embedded classes, or maybe adding target FIELD to ManagedExecutorDefinition:

@ApplicationScoped
public class ExecutorProducer {

    @ManagedExecutorDefinition(
        hungTaskThreshold = 120000,
        maxAsync = 5)
    @Produces
    @BackgroundJobs
    ManagedExecutorService executor;

    @ManagedExecutorDefinition(
        hungTaskThreshold = 10000,
        maxAsync = 1)
    @Produces
    @SequentialExecutor
    ManagedExecutorService executor;
}

[njr-11]

We may need to restrict the cases where qualifiers can be used for ManagedExecutorDefinition (and other definitions). We may use embedded classes, or maybe adding target FIELD to ManagedExecutorDefinition

I think restricting would be fine, and the example with ManagedExecutorDefinition on fields looks really nice. But then I realized, as you point out that @ManagedExecutorDefinition isn't currently allowed on FIELD, and that the definition annotations that it's copied from (DataSourceDefinition, ConnectionFactoryDefinition, and so forth) also don't allow FIELD. If we go for that pattern, I think we would want consistency across the platform at least with respect to allowing all definition annotations on FIELD.

What about using the first approach when there’s only one definition and the second when there are more definitions?

It seems complicated and confusing to have two different approaches and I don't think the first approach is clear even in the simple scenario. Unless it is restricted such as what Arjan proposed, how can it be known unambiguously whether or not annotations ought to be treated as qualifiers when they happen to appear alongside a ManagedExecutorDefinition versus having some completely different purpose?

If a qualifier is used with @ManagedExecutorDefinition, the name parameter should become optional, with a generated JNDI name if omitted.

+1

Also I would point out that support for @Resource continues to be needed because lots of code already exists which is currently using it. We can come up with better approaches and promote those as best practice and hopefully applications change over time, but I'd be hesitant to go as far as removing the @Resource capability. Let's not break frequently used patterns in existing applications without a really good reason behind it.

[njr-11]

For this issue, we should also decide if this constitutes a breaking change such that the next version of Concurrency should be 4.0 rather than 3.1. It seems to me that it does because a user could currently have a Producer of ManagedExecutorService (and the other types) without a qualifier and already be using the following for it,

@Inject
ManagedExecutorService executor;

Hopefully few if any users are actually doing that and always using their own qualifiers, in which case there will be no change to their applications, but it is possible to impact current behavior if they have done this without a qualifier.

[OndroMih]

For this issue, we should also decide if this constitutes a breaking change such that the next version of Concurrency should be 4.0 rather than 3.1. It seems to me that it does because a user could currently have a Producer of ManagedExecutorService (and the other types) without a qualifier and already be using the following for it,

@Inject
ManagedExecutorService executor;

I think that it's possible to define a bean by the container only if it's not already defined by the application. In the afterBeanDiscovery even handler, it should be possible to check whether a ManagedExecutorService bean with no qualifier is defined and create it if it's not. This should be also pretty easy to cover by the TCK. Then we don't need to break any existing applicatoins.

[njr-11]

I just realized there is another problem with:

@MyExecutor
@ManagedExecutorDefinition(...)
@ApplicationScoped
public class ExampleBean {

@Inject
@MyExecutor
ManagedExecutorService executor;

In addition to applying the MyExecutor qualifier to the ManagedExecutorDefinition, it also ends up being applied to the ApplicationScoped ExampleBean. It seems unlikely that user will want to apply the qualifier to both, so I don't think this mechanism ends up being workable even if we did limit it to a single resource definition.

[OndroMih]

True. It would rather have to be something like:

@ManagedExecutorDefinition(qualifiers = MyExecutor.class, ...)
@ApplicationScoped
public class ExampleBean {

[njr-11]

True. It would rather have to be something like:

@ManagedExecutorDefinition(qualifiers = MyExecutor.class, ...)
@ApplicationScoped
public class ExampleBean {

I created pull #348 to add the above pattern, along with the other part of this issue/proposal, which is the ability to inject the default ManagedExecutorService and so forth without qualifiers.

[arjantijms]

It seems unlikely that user will want to apply the qualifier to both

True, although how much does it matter? Typically those init / definition beans are not meant for injection anyway.

Of course the catch is in "typically", and I agree, sometimes the user would want such init bean to be injectable.