Affects: 5.3.28


Below is a test that is able to reproduce the issue.

import org.junit.jupiter.api.Test;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Scope;

import static org.assertj.core.api.Assertions.assertThat;

public class NotRequiredIssue {

    @Test
    void test() {
        AnnotationConfigApplicationContext ctx 
            = new AnnotationConfigApplicationContext(Bean1.class, Bean2Factory.class);

        Bean1 bean1A = ctx.getBean(Bean1.class);
        // Bean2Factory.bean2() returns null initially, so Bean1.bean2 got null injected
        assertThat(bean1A.bean2).isNull();

        Bean2 bean2 = new Bean2();

        Bean2Factory bean2Factory = ctx.getBean(Bean2Factory.class);
        // ask Bean2Factory.bean2() to return non-null instance
        bean2Factory.bean2 = bean2;

        Bean1 bean1B = ctx.getBean(Bean1.class);
        // expects Bean1.bean2 is non-null, however, still got null
        assertThat(bean1B.bean2).isNotNull();
        assertThat(bean1B.bean2).isSameAs(bean2);

        ctx.close();
    }

    @Scope("prototype")
    static class Bean1 {

        @Autowired(required = false) // required=false, otherwise will get exception 
        Bean2 bean2;

    }

    static class Bean2 {
    }

    static class Bean2Factory {

        Bean2 bean2;

        @Scope("prototype") @Bean
        Bean2 bean2() {
            return this.bean2;
        }
    }

}

Comment From: jhoeller

An initial note: The Bean2Factory class itself should not declare @Scope("prototype") in this case, otherwise anything set on it will not survive until the next Bean2 retrieval attempt since the factory instance is recreated every time.

With that sorted out, the actual null handling mismatch comes from an explicit check where null values for non-required injection points will indeed remain null forever. We'll have to revisit where that special check came from. Generally speaking, the null in that check is meant to indicate "not resolvable" (a permanent state) rather than "currently not present" (a temporary state), based on no matching bean defined at all as opposed to a matching factory method currently returning null.

For the time being, you could consider using Optional<Bean2> or ObjectProvider<Bean2> for your injection point instead. The required flag is not relevant then, the optional presence checks can happen programmatically on an always-injected handle. Design-wise, that's generally sensible since you do not have to rely on the ambiguous semantics of null.

Comment From: qiangyt

An initial note: The Bean2Factory class itself should not declare @Scope("prototype") in this case, otherwise anything set on it will not survive until the next Bean2 retrieval attempt since the factory instance is recreated every time.

With that sorted out, the actual null handling mismatch comes from an explicit check where null values for non-required injection points will indeed remain null forever. We'll have to revisit where that special check came from. Generally speaking, the null in that check is meant to indicate "not resolvable" (a permanent state) rather than "currently not present" (a temporary state), based on no matching bean defined at all as opposed to a matching factory method currently returning null.

For the time being, you could consider using Optional<Bean2> or ObjectProvider<Bean2> for your injection point instead. The required flag is not relevant then, the optional presence checks can happen programmatically on an always-injected handle. Design-wise, that's generally sensible since you do not have to rely on the ambiguous semantics of null.

Updated the test code.

And I do agree that Optional or ObjectProvider is better, my trouble is that this issue cames from some legacy injection code which a lot of codes depend on it and need to update to Optional<>.

Comment From: jhoeller

Related Issues

  • 30485

  • 26517

Comment From: jhoeller

Addressed through revised caching now, being able to deal with a switch from null to non-null and also the other way round.