Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AutoConfigurationSorter does not always respect @AutoConfigureOrder(Ordered.LOWEST_PRECEDENCE) #38904

Closed
philwebb opened this issue Dec 21, 2023 · 4 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@philwebb
Copy link
Member

See #38880 (comment)

If we fix this we may want to revert #38880 given this comment which I missed when apply the fix.

@philwebb
Copy link
Member Author

Repeating the comment from @alexandreBaronZnk here:


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 : #33291 (comment).
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
Copy link
Member Author

I've got a change lined up in https://github.com/philwebb/spring-boot/tree/gh-38904 but it's been quite hard to replicate the problem in a test.

@alexandreBaronZnk Do you have a project that replicates the ordering issues you were debugging that you could share?

@alexandreBaronZnk
Copy link

@philwebb Reproducing this behavior is what make me crazy since a couple of days.

Just for an exemple, on my business application, a version doesn't have the issue, and just adding a prometheus endpoint to override the one provided by actuator (to remove exemplars that are not supported by my prometheus version) produce the issue.
And with this buggy version, just remove the dependency spring-cloud-gcp-starter-trace fix it.

But I think that this unit test can reproduce it : main...alexandreBaronZnk:spring-boot-issue-38904:test-38904

@philwebb I hope it could be useful.

@philwebb philwebb changed the title Investigate why AutoConfigurationSorter does not always respect @AutoConfigureOrder(Ordered.LOWEST_PRECEDENCE) AutoConfigurationSorter does not always respect @AutoConfigureOrder(Ordered.LOWEST_PRECEDENCE) Dec 22, 2023
@philwebb
Copy link
Member Author

I hope it could be useful.

@alexandreBaronZnk You bet it's useful!! Thanks so much for your efforts with this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants