diff --git a/core/src/main/java/google/registry/model/registry/RegistryLockDao.java b/core/src/main/java/google/registry/model/registry/RegistryLockDao.java index 88e434587..1b88fff59 100644 --- a/core/src/main/java/google/registry/model/registry/RegistryLockDao.java +++ b/core/src/main/java/google/registry/model/registry/RegistryLockDao.java @@ -41,6 +41,7 @@ public final class RegistryLockDao { Long.class) .setParameter("verificationCode", verificationCode) .getSingleResult(); + // TODO(gbrodman): Don't throw NPE here. Maybe NoResultException fits better? checkNotNull(revisionId, "No registry lock with this code"); return em.find(RegistryLock.class, revisionId); }); diff --git a/core/src/main/java/google/registry/model/transaction/JpaTransactionManagerImpl.java b/core/src/main/java/google/registry/model/transaction/JpaTransactionManagerImpl.java index faab1e9e9..9d5e18042 100644 --- a/core/src/main/java/google/registry/model/transaction/JpaTransactionManagerImpl.java +++ b/core/src/main/java/google/registry/model/transaction/JpaTransactionManagerImpl.java @@ -79,17 +79,14 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { T result = work.run(); txn.commit(); return result; - } catch (Throwable transactionException) { - String rollbackMessage; + } catch (RuntimeException e) { try { txn.rollback(); - rollbackMessage = "transaction rolled back"; + logger.atWarning().log("Error during transaction; transaction rolled back"); } catch (Throwable rollbackException) { - logger.atSevere().withCause(rollbackException).log("Rollback failed, suppressing error"); - rollbackMessage = "transaction rollback failed"; + logger.atSevere().withCause(rollbackException).log("Rollback failed; suppressing error"); } - throw new PersistenceException( - "Error during transaction, " + rollbackMessage, transactionException); + throw e; } finally { txnInfo.clear(); } diff --git a/core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java b/core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java index c69203404..06d63fe90 100644 --- a/core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java +++ b/core/src/test/java/google/registry/model/registry/RegistryLockDaoTest.java @@ -25,7 +25,6 @@ import google.registry.schema.domain.RegistryLock.Action; import google.registry.testing.AppEngineRule; import java.util.Optional; import java.util.UUID; -import javax.persistence.PersistenceException; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -54,15 +53,11 @@ public final class RegistryLockDaoTest { public void testSaveAndLoad_failure_differentCode() { RegistryLock lock = createLock(); RegistryLockDao.save(lock); - PersistenceException exception = + NullPointerException thrown = assertThrows( - PersistenceException.class, + NullPointerException.class, () -> RegistryLockDao.getByVerificationCode(UUID.randomUUID().toString())); - assertThat(exception) - .hasCauseThat() - .hasMessageThat() - .isEqualTo("No registry lock with this code"); - assertThat(exception).hasCauseThat().isInstanceOf(NullPointerException.class); + assertThat(thrown).hasMessageThat().isEqualTo("No registry lock with this code"); } @Test diff --git a/core/src/test/java/google/registry/model/tmch/ClaimsListDaoTest.java b/core/src/test/java/google/registry/model/tmch/ClaimsListDaoTest.java index c238f8efe..17beda3df 100644 --- a/core/src/test/java/google/registry/model/tmch/ClaimsListDaoTest.java +++ b/core/src/test/java/google/registry/model/tmch/ClaimsListDaoTest.java @@ -22,7 +22,6 @@ import google.registry.model.transaction.JpaTransactionManagerRule; import google.registry.schema.tmch.ClaimsList; import google.registry.testing.FakeClock; import javax.persistence.NoResultException; -import javax.persistence.PersistenceException; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -71,9 +70,7 @@ public class ClaimsListDaoTest { @Test public void getCurrent_throwsNoResultExceptionIfTableIsEmpty() { - PersistenceException thrown = - assertThrows(PersistenceException.class, () -> ClaimsListDao.getCurrent()); - assertThat(thrown).hasCauseThat().isInstanceOf(NoResultException.class); + assertThrows(NoResultException.class, ClaimsListDao::getCurrent); } @Test diff --git a/core/src/test/java/google/registry/persistence/CurrencyUnitConverterTest.java b/core/src/test/java/google/registry/persistence/CurrencyUnitConverterTest.java index 6f1b5b97c..cca8315ed 100644 --- a/core/src/test/java/google/registry/persistence/CurrencyUnitConverterTest.java +++ b/core/src/test/java/google/registry/persistence/CurrencyUnitConverterTest.java @@ -75,11 +75,7 @@ public class CurrencyUnitConverterTest { jpaTm() .transact( () -> jpaTm().getEntityManager().find(TestEntity.class, "id").currency)); - assertThat(thrown) - .hasCauseThat() - .hasCauseThat() - .hasMessageThat() - .isEqualTo("Unknown currency 'XXXX'"); + assertThat(thrown).hasCauseThat().hasMessageThat().isEqualTo("Unknown currency 'XXXX'"); } @Entity(name = "TestEntity") // Override entity name to avoid the nested class reference. diff --git a/core/src/test/java/google/registry/schema/tld/PremiumListDaoTest.java b/core/src/test/java/google/registry/schema/tld/PremiumListDaoTest.java index 21e3d366e..1b37ad68e 100644 --- a/core/src/test/java/google/registry/schema/tld/PremiumListDaoTest.java +++ b/core/src/test/java/google/registry/schema/tld/PremiumListDaoTest.java @@ -21,7 +21,6 @@ import static google.registry.testing.JUnitBackports.assertThrows; import com.google.common.collect.ImmutableMap; import google.registry.model.transaction.JpaTransactionManagerRule; import java.math.BigDecimal; -import javax.persistence.PersistenceException; import org.joda.money.CurrencyUnit; import org.junit.Rule; import org.junit.Test; @@ -68,16 +67,13 @@ public class PremiumListDaoTest { @Test public void saveNew_throwsWhenPremiumListAlreadyExists() { PremiumListDao.saveNew(PremiumList.create("testlist", CurrencyUnit.USD, TEST_PRICES)); - PersistenceException thrown = + IllegalArgumentException thrown = assertThrows( - PersistenceException.class, + IllegalArgumentException.class, () -> PremiumListDao.saveNew( PremiumList.create("testlist", CurrencyUnit.USD, TEST_PRICES))); - assertThat(thrown) - .hasCauseThat() - .hasMessageThat() - .contains("A premium list of this name already exists"); + assertThat(thrown).hasMessageThat().contains("A premium list of this name already exists"); } @Test