You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by ma...@apache.org on 2016/12/02 08:46:44 UTC
ambari git commit: AMBARI-18956. Ambari attempts to commit
transactions marked as rollback-only (magyari_sandor)
Repository: ambari
Updated Branches:
refs/heads/trunk d24beb17e -> 73d372b8d
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/73d372b8
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/73d372b8
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/73d372b8
Branch: refs/heads/trunk
Commit: 73d372b8db6b4809fac98338aef1c04ae4c95c92
Parents: d24beb1
Author: Sandor Magyari <sm...@hortonworks.com>
Authored: Fri Dec 2 09:43:24 2016 +0100
Committer: Sandor Magyari <sm...@hortonworks.com>
Committed: Fri Dec 2 09:43:24 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/73d372b8/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/73d372b8/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();
+ }
+
+}