ambari-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From stoa...@apache.org
Subject ambari git commit: AMBARI-18956. Ambari attempts to commit transactions marked as rollback-only (magyari_sandor)
Date Mon, 19 Dec 2016 17:41:01 GMT
Repository: ambari
Updated Branches:
  refs/heads/branch-2.5 6512cc1a0 -> 4c04a9153


AMBARI-18956. Ambari attempts to commit transactions marked as rollback-only (magyari_sandor)


Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/4c04a915
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/4c04a915
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/4c04a915

Branch: refs/heads/branch-2.5
Commit: 4c04a91536f87c422440b4e954d810336585882d
Parents: 6512cc1
Author: Sandor Magyari <smagyari@hortonworks.com>
Authored: Fri Dec 2 09:43:24 2016 +0100
Committer: Toader, Sebastian <stoader@hortonworks.com>
Committed: Mon Dec 19 18:40:35 2016 +0100

----------------------------------------------------------------------
 .../orm/AmbariJpaLocalTxnInterceptor.java       |   9 +-
 .../orm/AmbariJpaLocalTxnInterceptorTest.java   | 155 +++++++++++++++++++
 2 files changed, 162 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/4c04a915/ambari-server/src/main/java/org/apache/ambari/server/orm/AmbariJpaLocalTxnInterceptor.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/AmbariJpaLocalTxnInterceptor.java
b/ambari-server/src/main/java/org/apache/ambari/server/orm/AmbariJpaLocalTxnInterceptor.java
index d7ba463..e19192a 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/AmbariJpaLocalTxnInterceptor.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/AmbariJpaLocalTxnInterceptor.java
@@ -218,12 +218,17 @@ public class AmbariJpaLocalTxnInterceptor implements MethodInterceptor
{
   /**
    * Returns True if rollback DID NOT HAPPEN (i.e. if commit should continue).
    *
-   * @param transactional The metadata annotaiton of the method
+   * @param transactional The metadata annotation of the method
    * @param e             The exception to test for rollback
    * @param txn           A JPA Transaction to issue rollbacks on
    */
-  private boolean rollbackIfNecessary(Transactional transactional, Exception e,
+  static boolean rollbackIfNecessary(Transactional transactional, Exception e,
                                       EntityTransaction txn) {
+    if (txn.getRollbackOnly()) {
+      txn.rollback();
+      return false;
+    }
+
     boolean commit = true;
 
     //check rollback clauses

http://git-wip-us.apache.org/repos/asf/ambari/blob/4c04a915/ambari-server/src/test/java/org/apache/ambari/server/orm/AmbariJpaLocalTxnInterceptorTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/orm/AmbariJpaLocalTxnInterceptorTest.java
b/ambari-server/src/test/java/org/apache/ambari/server/orm/AmbariJpaLocalTxnInterceptorTest.java
new file mode 100644
index 0000000..ea7ea53
--- /dev/null
+++ b/ambari-server/src/test/java/org/apache/ambari/server/orm/AmbariJpaLocalTxnInterceptorTest.java
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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
+ *
+ *     http://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 org.apache.ambari.server.orm;
+
+import com.google.inject.persist.Transactional;
+import org.easymock.EasyMockSupport;
+import org.junit.Assert;
+import org.junit.Test;
+
+import javax.persistence.EntityTransaction;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+
+import static org.apache.ambari.server.orm.AmbariJpaLocalTxnInterceptor.rollbackIfNecessary;
+import static org.easymock.EasyMock.expect;
+
+public class AmbariJpaLocalTxnInterceptorTest extends EasyMockSupport {
+
+  @Test
+  public void canBeCommittedIfExceptionsToBeRolledBackOnIsEmpty() {
+    Transactional transactional = createNiceMock(Transactional.class);
+    EntityTransaction transaction = createStrictMock(EntityTransaction.class);
+
+    expect(transaction.getRollbackOnly()).andReturn(false);
+    expect(transactional.rollbackOn()).andReturn(new Class[0]);
+
+    replayAll();
+
+    boolean canCommit = rollbackIfNecessary(transactional, new RuntimeException(), transaction);
+    Assert.assertTrue("Should be allowed to commit, since rollbackOn clause is empty", canCommit);
+
+    verifyAll();
+  }
+
+  @Test
+  public void canBeCommittedIfUnknownExceptionThrown() {
+    Transactional transactional = createNiceMock(Transactional.class);
+    EntityTransaction transaction = createStrictMock(EntityTransaction.class);
+
+    expect(transaction.getRollbackOnly()).andReturn(false);
+    expect(transactional.rollbackOn()).andReturn(new Class[] { IllegalArgumentException.class
});
+
+    replayAll();
+
+    boolean canCommit = rollbackIfNecessary(transactional, new RuntimeException(), transaction);
+    Assert.assertTrue("Should be allowed to commit, exception thrown does not match rollbackOn
clause", canCommit);
+
+    verifyAll();
+  }
+
+  @Test
+  public void rolledBackForKnownException() {
+    Transactional transactional = createNiceMock(Transactional.class);
+    EntityTransaction transaction = createStrictMock(EntityTransaction.class);
+
+    expect(transaction.getRollbackOnly()).andReturn(false);
+    expect(transactional.rollbackOn()).andReturn(new Class[] {
+      NullPointerException.class, IllegalArgumentException.class
+    });
+    expect(transactional.ignore()).andReturn(new Class[0]);
+    transaction.rollback();
+
+    replayAll();
+
+    boolean canCommit = rollbackIfNecessary(transactional, new IllegalArgumentException("rolling
back"), transaction);
+    Assert.assertFalse("Should be rolled back, since exception matches rollbackOn clause",
canCommit);
+
+    verifyAll();
+  }
+
+  @Test
+  public void rolledBackForSubclassOfKnownException() {
+    Transactional transactional = createNiceMock(Transactional.class);
+    EntityTransaction transaction = createStrictMock(EntityTransaction.class);
+
+    expect(transaction.getRollbackOnly()).andReturn(false);
+    expect(transactional.rollbackOn()).andReturn(new Class[] { RuntimeException.class });
+    expect(transactional.ignore()).andReturn(new Class[0]);
+    transaction.rollback();
+
+    replayAll();
+
+    boolean canCommit = rollbackIfNecessary(transactional, new IllegalArgumentException("rolling
back"), transaction);
+    Assert.assertFalse("Should be rolled back, since exception is subclass of the one in
rollbackOn clause", canCommit);
+
+    verifyAll();
+  }
+
+  @Test
+  public void canBeCommittedIfIgnoredExceptionThrown() {
+    Transactional transactional = createNiceMock(Transactional.class);
+    EntityTransaction transaction = createStrictMock(EntityTransaction.class);
+
+    expect(transaction.getRollbackOnly()).andReturn(false);
+    expect(transactional.rollbackOn()).andReturn(new Class[] { IllegalArgumentException.class
});
+    expect(transactional.ignore()).andReturn(new Class[] { NumberFormatException.class });
+
+    replayAll();
+
+    boolean canCommit = rollbackIfNecessary(transactional, new NumberFormatException("rolling
back"), transaction);
+    Assert.assertTrue("Should be allowed to commit, since ignored exception was thrown",
canCommit);
+
+    verifyAll();
+  }
+
+  @Test
+  public void canBeCommittedIfSubclassOfIgnoredExceptionThrown() {
+    Transactional transactional = createNiceMock(Transactional.class);
+    EntityTransaction transaction = createStrictMock(EntityTransaction.class);
+
+    expect(transaction.getRollbackOnly()).andReturn(false);
+    expect(transactional.rollbackOn()).andReturn(new Class[] { Exception.class });
+    expect(transactional.ignore()).andReturn(new Class[] { IOException.class });
+
+    replayAll();
+
+    boolean canCommit = rollbackIfNecessary(transactional, new FileNotFoundException("rolling
back"), transaction);
+    Assert.assertTrue("Should be allowed to commit, since subclass of ignored exception was
thrown", canCommit);
+
+    verifyAll();
+  }
+
+  @Test
+  public void rolledBackIfTransactionMarkedRollbackOnly() {
+    Transactional transactional = createNiceMock(Transactional.class);
+    EntityTransaction transaction = createStrictMock(EntityTransaction.class);
+
+    expect(transaction.getRollbackOnly()).andReturn(true);
+    transaction.rollback();
+
+    replayAll();
+
+    boolean canCommit = rollbackIfNecessary(transactional, null, transaction);
+    Assert.assertFalse("Should be rolled back, since transaction was marked rollback-only",
canCommit);
+
+    verifyAll();
+  }
+
+}


Mime
View raw message