Skip to content

Fix @Transactional examples regarding method visibility#27001

Merged
sbrannen merged 1 commit intospring-projects:mainfrom
hrybs:Fix-Transactional-annotation-description
May 30, 2021
Merged

Fix @Transactional examples regarding method visibility#27001
sbrannen merged 1 commit intospring-projects:mainfrom
hrybs:Fix-Transactional-annotation-description

Conversation

@hrybs
Copy link
Copy Markdown
Contributor

@hrybs hrybs commented May 29, 2021

Spring documentation "Data Access", paragraph "1.4.6. Using @Transactional" contains incorrect description of @Transactional annotation that is used on a class level and incorrect corresponding code snippets.

  • According to source codes of AnnotationTransactionAspect and ProxyTransactionManagementConfiguration the @Transactional annotation when used on a class level is only applied to public method even if we use AspectJ.
  • The are 2 incorrect code snippets that show class level @Transactional annotation for package-private methods.

@sbrannen sbrannen self-assigned this May 30, 2021
@sbrannen sbrannen added the type: documentation A documentation task label May 30, 2021
@sbrannen sbrannen added this to the 5.3.8 milestone May 30, 2021
@sbrannen sbrannen added the in: data Issues in data modules (jdbc, orm, oxm, tx) label May 30, 2021
@sbrannen sbrannen changed the title Fix @Transactional docs description Fix @Transactional docs regarding method visibility May 30, 2021
@sbrannen sbrannen merged commit 4c28266 into spring-projects:main May 30, 2021
@sbrannen
Copy link
Copy Markdown
Member

Good catch!

This has been merged into main.

Thanks

@sbrannen
Copy link
Copy Markdown
Member

the @Transactional annotation when used on a class level is only applied to public method even if we use AspectJ.

On second thought, that's not entirely true: it depends on the publicMethodsOnly flag in AnnotationTransactionAttributeSource.

When using @EnableTransactionManagement, ProxyTransactionManagementConfiguration.transactionAttributeSource() uses the default AnnotationTransactionAttributeSource constructor which sets publicMethodsOnly to true.

However, in the Spring TestContext Framework, the TransactionalTestExecutionListener sets the publicMethodsOnly flag to false.

In light of that, I'll modify the wording in the docs accordingly.

@sbrannen
Copy link
Copy Markdown
Member

As a proof of concept, I put together the following test case which demonstrates that @Transactional -- when declared solely at the class level -- can be configured to apply to all non-private methods in a standard Spring application as well.

import javax.sql.DataSource;

import org.junit.jupiter.api.Test;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.jdbc.datasource.DataSourceTransactionManager;
import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder;
import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
import org.springframework.transaction.PlatformTransactionManager;
import org.springframework.transaction.annotation.AnnotationTransactionAttributeSource;
import org.springframework.transaction.annotation.EnableTransactionManagement;
import org.springframework.transaction.annotation.ProxyTransactionManagementConfiguration;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.interceptor.TransactionAttributeSource;

import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.transaction.support.TransactionSynchronizationManager.isActualTransactionActive;

@SpringJUnitConfig
class NonPublicTxMethods {

	@Test
	void test(@Autowired Service service) {
		assertThat(isActualTransactionActive()).isFalse();
		service.doSomething();
	}

	@Configuration
	@EnableTransactionManagement
	static class Config {

		@Bean
		DataSource dataSource() {
			return new EmbeddedDatabaseBuilder().generateUniqueName(true).build();
		}

		@Bean
		PlatformTransactionManager transactionManager(DataSource dataSource) {
			return new DataSourceTransactionManager(dataSource);
		}

		/**
		 * Overrides {@link ProxyTransactionManagementConfiguration#transactionAttributeSource()}.
		 */
		@Bean
		TransactionAttributeSource transactionAttributeSource() {
			return new AnnotationTransactionAttributeSource(false);
		}

		@Bean
		Service service() {
			return new Service();
		}
	}

	@Transactional
	static class Service {

		void doSomething() {
			assertThat(isActualTransactionActive()).isTrue();
		}
	}

}

@hrybs
Copy link
Copy Markdown
Contributor Author

hrybs commented May 30, 2021

Good explanation! But when we use AspectJ the annotation is only applied to public methods.

@sbrannen
Copy link
Copy Markdown
Member

sbrannen commented May 30, 2021

But when we use AspectJ the annotation is only applied to public methods.

Yes, it's an unfortunate limitation of the predefined pointcuts in AnnotationTransactionAspect that limits the application to public methods when @Transactional is only declared at the class level.

However, non-public methods can at least be directly annotated with @Transactional when using AspectJ CTW or LTW without customization since AnnotationTransactionAspect uses new AnnotationTransactionAttributeSource(false).

@sbrannen sbrannen changed the title Fix @Transactional docs regarding method visibility Fix @Transactional examples regarding method visibility May 30, 2021
sbrannen pushed a commit that referenced this pull request May 30, 2021
@spring-projects-issues spring-projects-issues added the status: backported An issue that has been backported to maintenance branches label May 30, 2021
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this pull request Mar 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in: data Issues in data modules (jdbc, orm, oxm, tx) status: backported An issue that has been backported to maintenance branches type: documentation A documentation task

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants