[GitHub] WillemJiang closed pull request #321: SCB-963 Make saga and tcc callback method transactional

[GitBox <gi...@apache.org>]

WillemJiang closed pull request #321: SCB-963 Make saga and tcc callback method transactional
URL: https://github.com/apache/incubator-servicecomb-saga/pull/321
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/omega/omega-context/src/main/java/org/apache/servicecomb/saga/omega/context/CallbackContext.java b/omega/omega-context/src/main/java/org/apache/servicecomb/saga/omega/context/CallbackContext.java
index 0fe2613c..cf5ab508 100644
--- a/omega/omega-context/src/main/java/org/apache/servicecomb/saga/omega/context/CallbackContext.java
+++ b/omega/omega-context/src/main/java/org/apache/servicecomb/saga/omega/context/CallbackContext.java
@@ -35,9 +35,9 @@ public CallbackContext(OmegaContext omegaContext) {
     this.omegaContext = omegaContext;
   }
 
-  public void addCallbackContext(Method compensationMethod, Object target) {
+  public void addCallbackContext(String key, Method compensationMethod, Object target) {
     compensationMethod.setAccessible(true);
-    contexts.put(compensationMethod.toString(), new CallbackContextInternal(target, compensationMethod));
+    contexts.put(key, new CallbackContextInternal(target, compensationMethod));
   }
 
   public void apply(String globalTxId, String localTxId, String callbackMethod, Object... payloads) {
diff --git a/omega/omega-spring-tx/src/main/java/org/apache/servicecomb/saga/omega/transaction/spring/CompensableAnnotationProcessor.java b/omega/omega-spring-tx/src/main/java/org/apache/servicecomb/saga/omega/transaction/spring/CompensableAnnotationProcessor.java
index 790394f7..254c3e73 100644
--- a/omega/omega-spring-tx/src/main/java/org/apache/servicecomb/saga/omega/transaction/spring/CompensableAnnotationProcessor.java
+++ b/omega/omega-spring-tx/src/main/java/org/apache/servicecomb/saga/omega/transaction/spring/CompensableAnnotationProcessor.java
@@ -36,13 +36,13 @@
 
   @Override
   public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException {
-    checkMethod(bean);
-    checkFields(bean);
     return bean;
   }
 
   @Override
   public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException {
+    checkMethod(bean);
+    checkFields(bean);
     return bean;
   }
 
diff --git a/omega/omega-spring-tx/src/main/java/org/apache/servicecomb/saga/omega/transaction/spring/MethodCheckingCallback.java b/omega/omega-spring-tx/src/main/java/org/apache/servicecomb/saga/omega/transaction/spring/MethodCheckingCallback.java
index 8a2ac138..6c523134 100644
--- a/omega/omega-spring-tx/src/main/java/org/apache/servicecomb/saga/omega/transaction/spring/MethodCheckingCallback.java
+++ b/omega/omega-spring-tx/src/main/java/org/apache/servicecomb/saga/omega/transaction/spring/MethodCheckingCallback.java
@@ -18,11 +18,15 @@
 package org.apache.servicecomb.saga.omega.transaction.spring;
 
 import java.lang.invoke.MethodHandles;
+import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import org.apache.servicecomb.saga.omega.context.CallbackContext;
 import org.apache.servicecomb.saga.omega.transaction.OmegaException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.aop.framework.AdvisedSupport;
+import org.springframework.aop.framework.AopProxy;
+import org.springframework.aop.support.AopUtils;
 import org.springframework.util.ReflectionUtils.MethodCallback;
 
 public abstract class MethodCheckingCallback implements MethodCallback {
@@ -45,12 +49,50 @@ protected void loadMethodContext(Method method, String ... candidates) {
     for (String each : candidates) {
       try {
         Method signature = bean.getClass().getDeclaredMethod(each, method.getParameterTypes());
-        callbackContext.addCallbackContext(signature, bean);
+        String key = getTargetBean(bean).getClass().getDeclaredMethod(each, method.getParameterTypes()).toString();
+        callbackContext.addCallbackContext(key, signature, bean);
         LOG.debug("Found callback method [{}] in {}", each, bean.getClass().getCanonicalName());
-      } catch (NoSuchMethodException ex) {
+      } catch (Exception ex) {
         throw new OmegaException(
             "No such " + callbackType + " method [" + each + "] found in " + bean.getClass().getCanonicalName(), ex);
       }
     }
   }
+
+  private Object getTargetBean(Object proxy) throws Exception {
+    if(!AopUtils.isAopProxy(proxy)) {
+      return proxy;
+    }
+
+    if(AopUtils.isJdkDynamicProxy(proxy)) {
+      return getJdkDynamicProxyTargetObject(proxy);
+    } else {
+      return getCglibProxyTargetObject(proxy);
+    }
+  }
+
+  private Object getCglibProxyTargetObject(Object proxy) throws Exception {
+    Field h = proxy.getClass().getDeclaredField("CGLIB$CALLBACK_0");
+    h.setAccessible(true);
+    Object dynamicAdvisedInterceptor = h.get(proxy);
+
+    Field advised = dynamicAdvisedInterceptor.getClass().getDeclaredField("advised");
+    advised.setAccessible(true);
+
+    Object result = ((AdvisedSupport)advised.get(dynamicAdvisedInterceptor)).getTargetSource().getTarget();
+    return result;
+  }
+
+
+  private Object getJdkDynamicProxyTargetObject(Object proxy) throws Exception {
+    Field h = proxy.getClass().getSuperclass().getDeclaredField("h");
+    h.setAccessible(true);
+    AopProxy aopProxy = (AopProxy) h.get(proxy);
+
+    Field advised = aopProxy.getClass().getDeclaredField("advised");
+    advised.setAccessible(true);
+
+    Object result = ((AdvisedSupport)advised.get(aopProxy)).getTargetSource().getTarget();
+    return result;
+  }
 }
diff --git a/omega/omega-spring-tx/src/main/java/org/apache/servicecomb/saga/omega/transaction/spring/ParticipateAnnotationProcessor.java b/omega/omega-spring-tx/src/main/java/org/apache/servicecomb/saga/omega/transaction/spring/ParticipateAnnotationProcessor.java
index 5cef9ff4..80583bd2 100644
--- a/omega/omega-spring-tx/src/main/java/org/apache/servicecomb/saga/omega/transaction/spring/ParticipateAnnotationProcessor.java
+++ b/omega/omega-spring-tx/src/main/java/org/apache/servicecomb/saga/omega/transaction/spring/ParticipateAnnotationProcessor.java
@@ -36,13 +36,13 @@
 
   @Override
   public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException {
-    checkMethod(bean);
-    checkFields(bean);
     return bean;
   }
 
   @Override
   public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException {
+    checkMethod(bean);
+    checkFields(bean);
     return bean;
   }
 
diff --git a/omega/omega-spring-tx/src/test/java/org/apache/servicecomb/saga/omega/transaction/spring/TccInterceptorTest.java b/omega/omega-spring-tx/src/test/java/org/apache/servicecomb/saga/omega/transaction/spring/TccInterceptorTest.java
index bc9148b8..7f43476e 100644
--- a/omega/omega-spring-tx/src/test/java/org/apache/servicecomb/saga/omega/transaction/spring/TccInterceptorTest.java
+++ b/omega/omega-spring-tx/src/test/java/org/apache/servicecomb/saga/omega/transaction/spring/TccInterceptorTest.java
@@ -68,9 +68,6 @@
   @Autowired
   private List<String> messages;
 
-  @Autowired
-  private OmegaContext omegaContext;
-
   @Autowired
   private TccUserServiceMain tccUserServiceMain;
 
@@ -95,7 +92,6 @@ public void setUp() throws Exception {
   public void tearDown() throws Exception {
     messages.clear();
     userRepository.deleteAll();
-    omegaContext.clear();
   }
 
   @AfterClass
@@ -152,7 +148,8 @@ public void tccWorkflowFailed() {
     );
 
     User result = userRepository.findByUsername(user.username());
-    assertThat(result, is(nullValue()));
+    assertThat(result.username(), is(user.username()));
+    assertThat(result.email(), is(user.email()));
 
     result = userRepository.findByUsername(jack.username());
     assertThat(result, is(nullValue()));
diff --git a/omega/omega-spring-tx/src/test/java/org/apache/servicecomb/saga/omega/transaction/spring/TccUserService.java b/omega/omega-spring-tx/src/test/java/org/apache/servicecomb/saga/omega/transaction/spring/TccUserService.java
index 8223a874..31da4c9c 100644
--- a/omega/omega-spring-tx/src/test/java/org/apache/servicecomb/saga/omega/transaction/spring/TccUserService.java
+++ b/omega/omega-spring-tx/src/test/java/org/apache/servicecomb/saga/omega/transaction/spring/TccUserService.java
@@ -20,26 +20,28 @@
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
 
-import java.util.HashMap;
-import java.util.Map;
-
-import org.apache.servicecomb.saga.omega.context.OmegaContext;
+import javax.annotation.Resource;
 import org.apache.servicecomb.saga.omega.transaction.annotations.Participate;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
+import org.springframework.transaction.annotation.Transactional;
 
 @Component
-class TccUserService {
+@Transactional
+public class TccUserService {
   static final String ILLEGAL_USER = "Illegal User";
-  private final UserRepository userRepository;
+
 
   @Autowired
   TccUserService(UserRepository userRepository) {
     this.userRepository = userRepository;
   }
 
+  @Resource
+  private UserRepository userRepository;
+
   @Participate(confirmMethod = "confirm", cancelMethod = "cancel")
-  User add(User user) {
+  public User add(User user) {
     // Only for the validation check
     if (ILLEGAL_USER.equals(user.username())) {
       throw new IllegalArgumentException("User is illegal");
@@ -47,15 +49,14 @@ User add(User user) {
     return userRepository.save(user);
   }
 
-  void confirm(User user) {
+  public void confirm(User user) {
     User result = userRepository.findByUsername(user.username());
     // Just make sure we can get the resource and keep doing other business
     assertThat(result, is(user));
   }
 
-  void cancel(User user) {
+  public void cancel(User user) {
     userRepository.delete(user);
+    throw new RuntimeException("transaction test");
   }
-
-
 }
diff --git a/omega/omega-spring-tx/src/test/java/org/apache/servicecomb/saga/omega/transaction/spring/TransactionalUserService.java b/omega/omega-spring-tx/src/test/java/org/apache/servicecomb/saga/omega/transaction/spring/TransactionalUserService.java
index 0618109f..11bc4374 100644
--- a/omega/omega-spring-tx/src/test/java/org/apache/servicecomb/saga/omega/transaction/spring/TransactionalUserService.java
+++ b/omega/omega-spring-tx/src/test/java/org/apache/servicecomb/saga/omega/transaction/spring/TransactionalUserService.java
@@ -20,9 +20,11 @@
 import org.apache.servicecomb.saga.omega.transaction.annotations.Compensable;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
+import org.springframework.transaction.annotation.Transactional;
 
 @Component
-class TransactionalUserService {
+@Transactional
+public class TransactionalUserService {
   static final String ILLEGAL_USER = "Illegal User";
   private final UserRepository userRepository;
 
@@ -38,19 +40,19 @@ void resetCount() {
   }
 
   @Compensable(compensationMethod = "delete")
-  User add(User user) {
+  public User add(User user) {
     if (ILLEGAL_USER.equals(user.username())) {
       throw new IllegalArgumentException("User is illegal");
     }
     return userRepository.save(user);
   }
 
-  void delete(User user) {
+  public void delete(User user) {
     userRepository.delete(user);
   }
 
   @Compensable(retries = 2, compensationMethod = "delete")
-  User add(User user, int count) {
+  public User add(User user, int count) {
     if (this.count < count) {
       this.count += 1;
       throw new IllegalStateException("Retry harder");
@@ -59,7 +61,7 @@ User add(User user, int count) {
     return userRepository.save(user);
   }
 
-  void delete(User user, int count) {
+  public void delete(User user, int count) {
     resetCount();
     userRepository.delete(user);
   }


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services