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 <smagy...@hortonworks.com> Authored: Fri Dec 2 09:43:24 2016 +0100 Committer: Toader, Sebastian <stoa...@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(); + } + +}