Hi,

When I was writing custom DeferredImportSelector (https://github.com/spring-projects/spring-boot/pull/19400), I found an issue for handling DeferredImportSelector.Group with @Order on DeferredImportSelector.

Here is an example to describe the issue:

Let's say I have 3 DeferredImportSelectors with ordering specified 10, 20, 30. The one with order 10 and 30 returns same import-selector-group(GroupA), and the one with order 20 returns different import-selector-group(GroupB).

@Order(10)
static class DeferredImportSelectorA implements DeferredImportSelector {
  @Override
  public String[] selectImports(AnnotationMetadata importingClassMetadata) {
    return new String[]{MyConfigA.class.getName()};
  }

  @Override
  public Class<? extends Group> getImportGroup() {
    return GroupA.class;
  }
}

@Order(20)
static class DeferredImportSelectorB implements DeferredImportSelector {
  @Override
  public String[] selectImports(AnnotationMetadata importingClassMetadata) {
    return new String[]{MyConfigB.class.getName()};
  }

  @Override
  public Class<? extends Group> getImportGroup() {
    return GroupB.class;
  }
}

@Order(30)
static class DeferredImportSelectorC implements DeferredImportSelector {
  @Override
  public String[] selectImports(AnnotationMetadata importingClassMetadata) {
    return new String[]{MyConfigC.class.getName()};
  }

  @Override
  public Class<? extends Group> getImportGroup() {
    return GroupA.class;  // <== same group with selector-A
  }
}

It might be arguable to use same import-selector-group in differently ordered deferred import selector, but it is possible to write that way currently.

@Configuration(proxyBeanMethods = false)
@Import({DeferredImportSelectorA.class, DeferredImportSelectorB.class, DeferredImportSelectorC.class})
static class ImportConfig {
}

When ConfigurationClassParser parses this ImportConfig, I think expected order of returned ConfigurationClass are ordered by @Order: - ImportConfig - MyConfigA (from import-selector-A with order-10) - MyConfigB (from import-selector-B with order-20) - MyConfigC (from import-selector-C with order-30)

However, currently it returns this order: - ImportConfig - MyConfigA (from import-selector-A with order-10) - MyConfigC (from import-selector-C with order-30) <=== - MyConfigB (from import-selector-B with order-20)

This is because, when deferred-import-selectors are sorted, it orders selectors to selector-A, selector-B, selector-C based on the @Order which is correct. However, when Group is processed, since selector-A and selector-C uses same GroupA, when selector-A's group is processed, it also processes selector-C's imports as well. (here uses group as key)

This would be a problem, for example, selector-B is spring-boot's auto-configuration and selector-A and selector-C are to be applied before/after auto-configurations.

To fix this issue, in my patch, I have added DeferredImportSelectorGroupingKey for the LinkedHashMap that handles groupings. The key object also takes into account the order specified on import-selector. This way, even same import-selector-group is specified in import-selector with different order, they are considered to be in different group and the one has higher order priority is processed first. Of course, same order with same group will be treated in same category.

Comment From: snicoll

Can you share a bit more about your setup? DeferredImportSelector and the group feature in particular are meant to support Spring Boot auto-configurations and we didn't really expect user code would touch it.

Comment From: ttddyy

While trying to recall what it was, https://github.com/spring-projects/spring-boot/pull/19400 is the initial reason I was looking into the implementation of this area. I wanted to create annotations such as @ImportBeforeAutoConfiguration and @ImportAfterAutoConfiguration for my library to try processing configurations after user configurations but before/after auto-configurations, and giving that choice to the consumers of my library.

Initially, I tried to use the ordering on DeferredImportSelector with probably the same group that AutoConfigurationImportSelector uses (AutoConfigurationGroup), but because of the issue described in this ticket, probably the approach didn't work as expected. (I'm guessing what I did.) At the end, I have implemented DeferredImportSelector with custom Group classes that have relative before/after order from AutoConfigurationImportSelector.

I made @ImportBeforeAutoConfiguration and ImportAfterAutoConfiguration but they were too much internally involved and probably hard to maintain for other devs; so, we ended up just using @AutoConfigure[Before|After].

Comment From: snicoll

Thanks for the feedback.

I made @ImportBeforeAutoConfiguration and ImportAfterAutoConfiguration but they were too much internally involved and probably hard to maintain for other devs; so, we ended up just using @AutoConfigure[Before|After].

I think that's the right call. I am going to close this as I am not keen to bring this extra complexity.