Skip to content

Commit

Permalink
CORE-385 - Enable transaction management on package protected methods.
Browse files Browse the repository at this point in the history
The default AnnotationTransactionAttributeSource registered by Spring Framework is configured to *only* consider public methods for transaction attributes. This probably stems from the time of JDK proxies in which all invocable methods on the proxy are public by definition. In the age of CGLib proxies however, package protected methods can be invoked on the proxy and pass it properly. Due to the default setting within AnnotationTransactionAttributeSource, the interceptor chain ends up empty as it doesn't even bother to inspect methods for transaction annotations.

We introduce a workaround in the form of a PriorityOrdered BeanPostProcessor that flips the flag within AnnotationTransactionAttributeSource to false, so that the lookup of the transaction attribute also inspects package protected methods.

Aside: tweaked JpaAutoFlushTestExceutionCallback to only flush if a transactions is actually running, as that is the scenario it is supposed to handle: to make sure changes can be written to the database even in the light of a following rollback, in which the persistence provider would skip the writing entirely. We need this additional tweak as we need to run the test for the changes described above in a non-transactional integration test and trying to flush the EntityManager without a running transaction fails.
  • Loading branch information
odrotbohm committed Aug 12, 2020
1 parent 5fa4eba commit 4d4e823
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 0 deletions.
59 changes: 59 additions & 0 deletions backend/src/main/java/quarano/Quarano.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@
import quarano.core.web.MappingCustomizer;
import quarano.core.web.RepositoryMappingModule;

import java.lang.reflect.Field;
import java.util.List;
import java.util.Map;
import java.util.Properties;

import org.modelmapper.ModelMapper;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.config.BeanPostProcessor;
import org.springframework.boot.Banner;
import org.springframework.boot.ResourceBanner;
import org.springframework.boot.SpringApplication;
Expand All @@ -23,6 +26,7 @@
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Profile;
import org.springframework.context.support.MessageSourceAccessor;
import org.springframework.core.PriorityOrdered;
import org.springframework.core.convert.ConversionService;
import org.springframework.core.env.Environment;
import org.springframework.core.env.MutablePropertySources;
Expand All @@ -34,9 +38,13 @@
import org.springframework.core.io.support.PropertiesLoaderUtils;
import org.springframework.data.jpa.repository.config.EnableJpaAuditing;
import org.springframework.data.repository.support.Repositories;
import org.springframework.lang.Nullable;
import org.springframework.scheduling.annotation.EnableScheduling;
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.AnnotationTransactionAttributeSource;
import org.springframework.util.ReflectionUtils;

/**
* @author Oliver Drotbohm
Expand Down Expand Up @@ -98,6 +106,57 @@ MessageSourceAccessor messageSourceAccessor(MessageSource source) {
return new MessageSourceAccessor(source);
}

/**
* {@link BeanPostProcessor} to make sure package protected methods annotate with {@code Transactional}. We tweak the
* {@code publicMethodsOnly} flag inside {@link AnnotationTransactionAttributeSource} to {@literal false}.
*
* @author Oliver Drotbohm
*/
@Component
private static final class PackageProtectedTransactionMethodsProcessor implements BeanPostProcessor, PriorityOrdered {

private static final Field PUBLIC_METHODS_ONLY_FIELD;

static {

PUBLIC_METHODS_ONLY_FIELD = ReflectionUtils.findField(AnnotationTransactionAttributeSource.class,
"publicMethodsOnly");

if (PUBLIC_METHODS_ONLY_FIELD == null) {
throw new IllegalStateException(
"Could not find publicMethodsOnly field on AnnotationTransactionAttributeSource!");
}

ReflectionUtils.makeAccessible(PUBLIC_METHODS_ONLY_FIELD);
}

/*
* (non-Javadoc)
* @see org.springframework.beans.factory.config.BeanPostProcessor#postProcessAfterInitialization(java.lang.Object, java.lang.String)
*/
@Nullable
@Override
public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException {

if (!AnnotationTransactionAttributeSource.class.isInstance(bean)) {
return bean;
}

ReflectionUtils.setField(PUBLIC_METHODS_ONLY_FIELD, bean, false);

return bean;
}

/*
* (non-Javadoc)
* @see org.springframework.core.Ordered#getOrder()
*/
@Override
public int getOrder() {
return 100;
}
}

/**
* {@link Banner} implementation to include additional properties in the banner output.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.junit.jupiter.api.extension.ExtensionContext;
import org.springframework.context.ApplicationContext;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.transaction.support.TransactionSynchronizationManager;

/**
* A JUnit 5 callback to make sure, JPA {@link EntityManager}s contained in the application are flushed for each test
Expand All @@ -22,6 +23,10 @@ class JpaAutoFlushTestExecutionCallback implements AfterTestExecutionCallback {
@Override
public void afterTestExecution(ExtensionContext context) throws Exception {

if (!TransactionSynchronizationManager.isActualTransactionActive()) {
return;
}

ApplicationContext applicationContext = SpringExtension.getApplicationContext(context);

applicationContext.getBeanProvider(EntityManager.class)
Expand Down
31 changes: 31 additions & 0 deletions backend/src/test/java/quarano/SampleComponent.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright 2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package quarano;

import static org.assertj.core.api.Assertions.*;

import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.support.TransactionSynchronizationManager;

@Component
public class SampleComponent {

@Transactional
void someMethod() {
assertThat(TransactionSynchronizationManager.isActualTransactionActive()).isTrue();
}
}
38 changes: 38 additions & 0 deletions backend/src/test/java/quarano/TransactionSetupTests.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright 2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package quarano;

import lombok.RequiredArgsConstructor;

import org.junit.jupiter.api.Test;
import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional;

/**
* @author Oliver Drotbohm
*/
@RequiredArgsConstructor
@QuaranoIntegrationTest
@Transactional(propagation = Propagation.NOT_SUPPORTED)
public class TransactionSetupTests {

private final SampleComponent foo;

@Test
void createsTransactionForPackageProtectedMethod() {
foo.someMethod();
}
}

0 comments on commit 4d4e823

Please sign in to comment.