spring-projects / spring-boot

Spring Boot helps you to create Spring-powered, production-grade applications and services with absolute minimum fuss.
https://spring.io/projects/spring-boot
Apache License 2.0
75.23k stars 40.7k forks source link

HibernateJpaAutoConfiguration should be applied before DataSourceTransactionManagerAutoConfiguration #38880

Closed philwebb closed 11 months ago

philwebb commented 11 months ago

Forward port of issue #38861 to 3.2.x.

alexandreBaronZnk commented 11 months ago

@philwebb

Hi, I've the same issue described here #38861.

The solution to explicitly add a dependency between HibernateJpaConfiguration and DataSourceTransactionManagerAutoConfiguration is enough for this particular case.

But I think there is a more global problem in the AutoConfigurationSorter. There is a comment in an another issue for the same bug here : https://github.com/spring-projects/spring-boot/issues/33291#issuecomment-1323435208. It indicates that the @AutoConfigureOrder(Ordered.LOWEST_PRECEDENCE) should be enough to indicate that HibernateJpaConfiguration will be execute before DataSourceTransactionManagerAutoConfiguration.

The @AutoConfigureOrder(Ordered.LOWEST_PRECEDENCE) works like a charm when there is no constraint to TransactionAutoConfiguration in the current case. Because in the AutoConfigurationSorter the following method sort by order

    List<String> getInPriorityOrder(Collection<String> classNames) {
        AutoConfigurationClasses classes = new AutoConfigurationClasses(this.metadataReaderFactory,
                this.autoConfigurationMetadata, classNames);
        List<String> orderedClassNames = new ArrayList<>(classNames);
        // Initially sort alphabetically
        Collections.sort(orderedClassNames);
        // Then sort by order
        orderedClassNames.sort((o1, o2) -> {
            int i1 = classes.get(o1).getOrder();
            int i2 = classes.get(o2).getOrder();
            return Integer.compare(i1, i2);
        });
        // Then respect @AutoConfigureBefore @AutoConfigureAfter
        orderedClassNames = sortByAnnotation(classes, orderedClassNames);
        return orderedClassNames;
    }

But when there is a constraint, the recursive flow doesn't respect ordering.

    private void doSortByAfterAnnotation(AutoConfigurationClasses classes, List<String> toSort, Set<String> sorted,
            Set<String> processing, String current) {
        if (current == null) {
            current = toSort.remove(0);
        }
        processing.add(current);
        for (String after : classes.getClassesRequestedAfter(current)) {
            checkForCycles(processing, current, after);
            if (!sorted.contains(after) && toSort.contains(after)) {
                doSortByAfterAnnotation(classes, toSort, sorted, processing, after);
            }
        }
        processing.remove(current);
        sorted.add(current);
    }

The method getClassesRequestAfter use a HashMap to iterate over autoconfiguration, that make the result not previsible.

        private final Map<String, AutoConfigurationClass> classes = new HashMap<>();

        Set<String> getClassesRequestedAfter(String className) {
            Set<String> classesRequestedAfter = new LinkedHashSet<>(get(className).getAfter());
            this.classes.forEach((name, autoConfigurationClass) -> {
                if (autoConfigurationClass.getBefore().contains(className)) {
                    classesRequestedAfter.add(name);
                }
            });
            return classesRequestedAfter;
        }

And it is strange that the ordering in the after annotation impacts the final autoconfiguration ordering too.

A debugging in my application for the following instruction classes.getClassesRequestedAfter("org.springframework.boot.autoconfigure.transaction.TransactionAutoConfiguration") produces this result

org.springframework.boot.autoconfigure.transaction.TransactionManagerCustomizationAutoConfiguration,
org.springframework.boot.autoconfigure.transaction.jta.JtaAutoConfiguration,
org.springframework.boot.autoconfigure.jdbc.DataSourceTransactionManagerAutoConfiguration,
com.google.cloud.spring.autoconfigure.firestore.FirestoreTransactionManagerAutoConfiguration,
com.google.cloud.spring.autoconfigure.datastore.DatastoreTransactionManagerAutoConfiguration,
org.springframework.boot.autoconfigure.orm.jpa.HibernateJpaAutoConfiguration,
com.google.cloud.spring.autoconfigure.spanner.SpannerTransactionManagerAutoConfiguration

The DatastoreTransactionManagerAutoConfiguration is before HibernateJpaAutoConfiguration and this order is important in the final autoconfiguration ordering.

IHMO, the current fix should be enough to this particular case, but a more global fix should be done in the AutoConfigurationSorter class.

philwebb commented 10 months ago

@alexandreBaronZnk I've opened #38904, can we continue the discussion there?