Unexpected empty collection on Named collection type

[pault-t-canva]

Affects: 5.3.15

As introduced by #19901 and mentioned in #22735 it is expected that a collection type in a factory constructor provides an empty collection. Thus the following will occur

@Configuration 
class SampleConfiguration {
   @Bean 
   public TestClass testClass(Set<String> sampleSet) {
      return new TestClass(sampleSet); // In this case sampleSet will be an empty set if not found
  }
}

The suggested workaround if that was not desirable, was to check for empty methods and fail out.

In the following - more explicit - situation

@Configuration 
class SampleConfiguration {
   @Bean 
   public TestClass testClass(@Named("explicitName") Set<String> sampleSet) {
      return new TestClass(sampleSet); // In this case sampleSet will be an empty set if not found
  }
}

the sampleSet has been explicitly named. In this case, we will still receive an empty collection in the case that the bean cannot be found. This is surprising as at this point there is an explicit ask for a specific bean name. This makes a possible spelling mistake in the named bean have a high impact - because if an empty collection is valid, there's no way to tell if the bean was wired via the spring context, or if the default bean took its place.

An expected behaviour could be that if a bean was explicitly named - that it would fail in the event that the bean name was not found. Or that there would be some way to make the fallback passed though to https://github.com/spring-projects/spring-framework/blob/main/spring-beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java#L852 false if explicitly desireable (such as via another annotation).

[funky-eyes]

I support the idea that when the display indicates the collection bean I want, if the corresponding bean cannot be found, should throw an exception to terminate the run

[jdcaperon]

This would be great to have +1 for support

[snicoll]

I am afraid this is by design as you've figured it out yourself already and @Named is a qualifier. You're asking for a collection of beans of a certain type and the container is giving you exactly that.

[pault-t-canva]

Thanks @snicoll for adding clarity.

If something were to be considered in the future - I think a default to the current behaviour is totally fair enough - but a 'strict mode' type of setup would be appealing in certain situations. This would mean that the current behaviour could continue while those who would like to may opt into a more consistent behaviour with failure-to-wire erroring that occurs more broadly within a spring context.

An annotation on a module or bean would be wonderful to allow the best of both worlds, at the cost of increasing the API surface area.

@Configuration(strictCollections = true) // scuse my poor naming
// or 
@StrictCollections
class SampleConfiguration {
   @Bean 
   public TestClass testClass(@Named("explicitName") Set<String> sampleSet) {
      return new TestClass(sampleSet); // In this case sampleSet will be an empty set if not found
  }
}

[snicoll]

@pault-t-canva What is the @Named doing here? In the example above, I am assuming that you have multiple beans of type String and you're asking the context to gather them at the injection point. I am a bit confused as what "explicitName" refers to.

[pault-t-canva]

Trying to create a proper test case demonstrates there's some setup in my main project making @Named perform as @Qualified does.

Regardless, the following demonstrates the use. Rather than collecting strings, the use of the name is for disambiguating between multiple collections. Here's a similar example

@Configuration
public class TestConfig {

    @Bean("firstList")
    public List<String> firstList() {
        return List.of("hello", "there");
    }

    @Bean("secondList")
    public List<String> secondList() {
        return List.of("another", "list");
    }

    @Bean("finalList")
    public List<String> finalList(@Qualifier("secondList2") List<String> second) {
        var results = new ArrayList<>(second);
        results.add("final one");
        return results;
    }
}

fun main(args: Array<String>) {
  val ctx = AnnotationConfigApplicationContext()
  ctx.register(TestConfig::class.java)
  ctx.refresh()
  println(ctx.getBean("finalList"))
 // prints ["final one"] in the current case
 // with a correct qualifier prints
 // ["another", "list", "final one"]
}

In the above example, it would be nice if there were a mechanism (perhaps not the default) to fail in the event that the qualifier was unable to be found. Instead it's an empty list.

[snicoll]

Thanks for the follow-up, I better understand the picture now. It looks like you're using @Qualifier as a way to do by-name semantic but that's not what it's doing. It's really is a by-type, first with a more narrowed match if necessary.

If the qualifier doesn't match, it should not prevent the injection to happen, and it's going to do what you're asking here which is collecting the bean of type String in the context and inject that. If there are no beans, then it will inject an empty list per #19901

[pault-t-canva]

Yeah, as noted in the initial description - i completely understand and accept this is by design. The only question was whether there should be a way in which you can have this fail to build out a spring context via some sort of strict mode (so that current behaviour is not lost).

It sounds the the answer is no. Or more broadly that using named collections (and then qualifying them to disambiguate) is not a preferred strategy? And in these cases, because it doesn't find a List<String> with the qualifier, it searches for any beans of String that match the qualifier, and due to there being zero - it doesn't match. That behaviour is clearly a good thing as it allows for dynamic lists of beans to exist from zero to many.

Given that a strict type mode is not expected to be something that's reasonable, is the preferred strategy here to perform type wrapping of these collection types when you aren't attempting to collect all beans of a particular type from the context but rather a specific collection that's been pre-prepare?

For instance, rewriting the above example

@Configuration
public class TestConfig {

    public record FirstList(List<String> value) {
    }

    public record SecondList(List<String> value) {
    }

    @Bean("firstList")
    public FirstList firstList() {
        return new FirstList(List.of("hello", "there"));
    }

    @Bean("secondList")
    public SecondList secondList() {
        return new SecondList(List.of("another", "list"));
    }

    @Bean("finalList")
    public List<String> finalList(SecondList second) {
        var results = new ArrayList<>(second.value);
        results.add("final one");
        return results;
    }
}

[snicoll]

i completely understand and accept this is by design.

Given the examples you've shared, I am also trying to provide a comprehensive answer for anyone looking at this issue and wondering why it behaves this way.

The only question was whether there should be a way in which you can have this fail to build out a spring context via some sort of strict mode

I think it should be pretty clear by now that the answer is no.

is the preferred strategy here to perform type wrapping of these collection types when you aren't attempting to collect all beans of a particular type from the context but rather a specific collection that's been pre-prepare?

We should probably have started with that, indeed. Exposing a bean of type List or Map is really not recommended irrespective of what you're trying to do. Containers is really the way to go for this and many other use cases that involve manipulating multiples instances of a given type.

[pault-t-canva]

Fantastic, i hope that this issue is a helpful one for folks in the future.

I think the clear outcome is that using a collection as a return type from a bean method has got certain unexpected outcomes depending on your understanding of how the bean is eventually resolved.

If anything, there's the potential (and maybe i just didn't find it) that public docs could be helpful here. Regardless, i'm satisfied with the conclusion and appreciate your time.