[servicecomb-pack] 03/05: SCB-1385 Polish the code with suggestion from PR review

[ni...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ningjiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-pack.git

commit 72b93093f72d1db117a568c7231fed0488e53ec4
Author: Willem Jiang <wi...@gmail.com>
AuthorDate: Sat Jul 20 23:00:39 2019 +0800

    SCB-1385 Polish the code with suggestion from PR review
---
 .../pack/omega/transaction/TransactionAspect.java           | 13 +++++++++++--
 .../pack/omega/transaction/TransactionAspectTest.java       | 12 ++++++++++--
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/omega/omega-transaction/src/main/java/org/apache/servicecomb/pack/omega/transaction/TransactionAspect.java b/omega/omega-transaction/src/main/java/org/apache/servicecomb/pack/omega/transaction/TransactionAspect.java
index fa7bbf4..e7b3425 100644
--- a/omega/omega-transaction/src/main/java/org/apache/servicecomb/pack/omega/transaction/TransactionAspect.java
+++ b/omega/omega-transaction/src/main/java/org/apache/servicecomb/pack/omega/transaction/TransactionAspect.java
@@ -51,8 +51,13 @@ public class TransactionAspect {
     // just check if we need to setup the transaction context information first
     TransactionContext transactionContext = getTransactionContextFromArgs(joinPoint.getArgs());
     if (transactionContext != null) {
-      LOG.debug("Updated context {} for globalTxId:{} and localTxId:{}", context,
-          transactionContext.globalTxId(), transactionContext.localTxId());
+      if (context.globalTxId() != null) {
+        LOG.warn("The context {}'s globalTxId is not empty. Update it for globalTxId:{} and localTxId:{}", context,
+            transactionContext.globalTxId(), transactionContext.localTxId());
+      } else {
+        LOG.debug("Updated context {} for globalTxId:{} and localTxId:{}", context,
+            transactionContext.globalTxId(), transactionContext.localTxId());
+      }
       context.setGlobalTxId(transactionContext.globalTxId());
       context.setLocalTxId(transactionContext.localTxId());
     }
@@ -74,6 +79,10 @@ public class TransactionAspect {
   TransactionContext getTransactionContextFromArgs(Object[] args) {
     if (args != null) {
       for (Object arg : args) {
+        // check the TransactionContext first
+        if (arg instanceof TransactionContext) {
+          return (TransactionContext) arg;
+        }
         if (arg instanceof TransactionContextWrapper) {
           return ((TransactionContextWrapper) arg).getTransactionContext();
         }
diff --git a/omega/omega-transaction/src/test/java/org/apache/servicecomb/pack/omega/transaction/TransactionAspectTest.java b/omega/omega-transaction/src/test/java/org/apache/servicecomb/pack/omega/transaction/TransactionAspectTest.java
index 17a5aa3..c39caf2 100644
--- a/omega/omega-transaction/src/test/java/org/apache/servicecomb/pack/omega/transaction/TransactionAspectTest.java
+++ b/omega/omega-transaction/src/test/java/org/apache/servicecomb/pack/omega/transaction/TransactionAspectTest.java
@@ -46,6 +46,7 @@ import org.aspectj.lang.ProceedingJoinPoint;
 import org.aspectj.lang.reflect.MethodSignature;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Mockito;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
@@ -117,14 +118,21 @@ public class TransactionAspectTest {
   @Test
   public void testGetTransactionContextFromArgs() throws Throwable {
 
-    TransactionContext result = aspect.getTransactionContextFromArgs(new Object[]{tx,wrapper});
+    TransactionContext result = aspect.getTransactionContextFromArgs(new Object[]{wrapper});
     assertThat(result, is(tx));
 
     result = aspect.getTransactionContextFromArgs(new Object[]{});
     assertNull(result);
 
-    result = aspect.getTransactionContextFromArgs(new Object[]{tx});
+    result = aspect.getTransactionContextFromArgs(null);
     assertNull(result);
+
+    result = aspect.getTransactionContextFromArgs(new Object[]{tx});
+    assertThat(result, is(tx));
+
+    TransactionContext otherTx = Mockito.mock(TransactionContext.class);
+    result = aspect.getTransactionContextFromArgs(new Object[]{otherTx, wrapper});
+    assertThat(result, is(otherTx));
   }
 
   @Test