Add support for optional field/method injection

[graemerocher]

Currently we don't support optional injection of fields and methods. These are always injected and the only way to prevent a NoSuchBeanException is to use @Nullable but that results in injecting null.

To resolve this, this PR adds a @Autowired annotation which differs in behaviour from @Inject in that it has a required member that can be set to false. This mirrors the behaviour in Spring.

Looking elsewhere Guice provides their own @Inject annotation which supports an optional member https://google.github.io/guice/api-docs/4.2/javadoc/com/google/inject/Inject.html

I considered defining our own @Inject annotation and/or creating a new annotation but frankly having @Autowired made the most sense as it will also help users who are coming from Spring.

This PR also documents the injection types better and explains the possible ways to inject a bean.

[dstepanov]

Is there something that tests setting the null?

[graemerocher]

let me add more tests

[dstepanov]

Maybe we can just avoid setting null without a new annotation

[graemerocher]

that would be a breaking change in behaviour

[dstepanov]

I don’t like the idea of introducing a new inject annotation, maybe we can keep this hidden property for remapping Guava and change the behavior in v5?

[graemerocher]

I am fine to put this behind a meta annotation or something but how would you suggest the behaviour change? Currently @Nullable is not suitable because what if you want the field to not be nullable?

[dstepanov]

I would keep current behavior and introduce an internal parameter of the inject annotation to skip null fields setting, that parameter will be used only for Guava inject remapper and default to in v5

[graemerocher]

but we can't default to @Inject(required=false) because that would not fail when you want it to 🤔

[dstepanov]

For Guava? What is the problem remapping it to nullable and inject?

[dstepanov]

Maybe I don’t get something

[graemerocher]

The two cases are not semantically the same. I don't see how this case can be added without a new annotation. Let's look at with some examples.

Case A (Using @Nullable)

Say you have:

@Inject
@Nullable
Foo foo;

void someMethod() {
   if (foo != null) { // required null check because it is nullable
        foo.doSomething();
   }
}

Today if no bean exists this results in null getting injected. This is ok because foo is @Nullable and the behaviour is consistent for constructor, field and method injection. Since @Nullable is specified the code and IDE and static analysis tools provide warnings where foo is null and you must add a null check somewhere in the code.

Case B (Using @NonNull and required injection)

Say you have:

@Inject
@NonNull
Foo foo = new Foo();

void someMethod() {
        foo.doSomething();
}

Type is annotated with @NonNull and defaults to a new instance. No null checks needed, in fact @NonNull is specified so code can safely use foo safe in the knowledge that is not null. However, if Foo doesn't exist as a bean then NoSuchBeanException is thrown.

Case C (Using @NonNull and required injection)

Say you have:

@Inject(required=false)
@NonNull
Foo foo = new Foo();

void someMethod() {
        foo.doSomething();
}

Type is annotated with @NonNull and defaults to a new instance, you set required=false somewhere (new annotation? new annotation member?) to specify that if the bean doesn't exist don't inject and don't fail. The code can be safely written without null checks and we know it will never be null. Here is the case we do not support today.

Possible Solutions

1. Change the behaviour to not inject null which is what you suggest, but that will still fail Case C because foo is marked as @NonNull. It is also a breaking change in behaviour which will have upgrade implications and break many applications that today expect null to be injected
2. Introduce a new annotation (could be named whatever we want, I chose @Autowired). Downside is it is a new annotation, good news it breaks nobodies applications.

[sdelamo]

I have split the documentation in sections

Possible Solutions

1. Change the behaviour to not inject null which is what you suggest, but that will still fail Case C because foo is marked as @NonNull. It is also a breaking change in behaviour which will have upgrade implications and break many applications that today expect null to be injected

We should not do this.

[sdelamo]

I am not sure about this change. Adding a new annotation @Autowired may add more confusion than value.

However, the documentation improvements added in this PR are great, and I have moved them to a separate PR so that we can merge them independently of this discussion.

I think we already have two ways to achieve this:

Using @Secondary

For me, if you want a default implementation of a bean to be injected, if no other bean of a certain type is present you could leverage the @Secondary annotation.

You could have

package com.example;

public interface CsvParser {
    void parse();
}

package com.example;

import io.micronaut.context.annotation.Secondary;
import jakarta.inject.Singleton;

@Singleton
@Secondary
public class DefaultCsvParser implements CsvParser {
    @Override
    public void parse() {

    }
}

and you know your injection point is always not null:

package com.example;

import io.micronaut.core.annotation.NonNull;
import io.micronaut.http.HttpStatus;
import io.micronaut.http.annotation.Controller;
import io.micronaut.http.annotation.Get;
import io.micronaut.http.annotation.Status;
import jakarta.inject.Inject;

@Controller
class FooController {

    @Inject
    @NonNull
    CsvParser parser;

    @Get
    @Status(HttpStatus.OK)
    void index() {
        parser.parse();
    }
}

Using @PostConstruct

As you described in the docs improvements, another option is to use @PostConstruct.

You could have:

package com.example;

public interface CsvParser {
    void parse();
}

and an implementation which is not a bean:

package com.example;

public class DefaultCsvParser implements CsvParser {
    @Override
    public void parse() {

    }
}

and populated it only if the injection point is not fullfiled

package com.example;

import io.micronaut.core.annotation.Nullable;
import io.micronaut.http.HttpStatus;
import io.micronaut.http.annotation.Controller;
import io.micronaut.http.annotation.Get;
import io.micronaut.http.annotation.Status;
import jakarta.annotation.PostConstruct;
import jakarta.inject.Inject;

@Controller
class FooController {

    @Inject
    @Nullable
    CsvParser parser;

    @PostConstruct
    void init() {
        if (parser == null) {
            parser = new DefaultCsvParser();
        }
    }

    @Get
    @Status(HttpStatus.OK)
    void index() {
        parser.parse();
    }
}

IntelliJ IDEA understands parser is never null but if the user is bothered because parser is annotated as @Nullable and you invoke a method on it. The user could always create an access method with a return type annotated with @NonNull and access via it.

package com.example;

import io.micronaut.core.annotation.NonNull;
import io.micronaut.core.annotation.Nullable;
import io.micronaut.http.HttpStatus;
import io.micronaut.http.annotation.Controller;
import io.micronaut.http.annotation.Get;
import io.micronaut.http.annotation.Status;
import jakarta.annotation.PostConstruct;
import jakarta.inject.Inject;

@Controller
class FooController {

    @Inject
    @Nullable
    CsvParser parser;

    @PostConstruct
    void init() {
        if (parser == null) {
            parser = new DefaultCsvParser();
        }
    }

    @Get
    @Status(HttpStatus.OK)
    void index() {
        getParser().parse();
    }
    
    @NonNull
    CsvParser getParser() {
        return parser;
    }
}

Conclusion

I think the use of @Secondary is specially suited for this use case. I am not sure adding another way of doing field injection for this niche use case is worth it.

[graemerocher]

ok I will remove the annotation and make a meta-thing to support the Guice/Spring equivalent and there will be no way to access the behaviour from Micronaut itself.

[sdelamo]

Another way is to use the @DefaultImplementation specially after the improvements you did in #10820

[graemerocher]

Whilst I agree there are ways all of them force you to define an implementation somewhere. There is no way to do it just in code and all of the options you presented add more overhead and complication to the implementation. It also makes the code harder to test because instantiating the type also requires then setting the field to something and if the field is not public that is complicated.

However, field injection is not great practise anyway so I am not going to die on this hill trying to argue for this change as I don't care enough about it.

[graemerocher]

@sdelamo @dstepanov I have removed the new annotation and left the ability to define it via meta-annotation so the Micronaut Spring and upcoming Micronaut Guice projects can behave correctly if they choose to.

[sdelamo]

should not we delete this file if we are not showing it in the documentation?

[sonarcloud]

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
88.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud