Convert RegistrarTest to working with Postgresql (#865)

* Convert RegistrarTest to working with Postgresql

* Resolve comments
This commit is contained in:
Shicong Huang 2020-11-16 12:18:28 -05:00 committed by GitHub
parent 51942fcaad
commit c8159e7b35
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 89 additions and 75 deletions

View file

@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Strings.emptyToNull; import static com.google.common.base.Strings.emptyToNull;
import static com.google.common.base.Strings.nullToEmpty; import static com.google.common.base.Strings.nullToEmpty;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.ImmutableSortedMap.toImmutableSortedMap; import static com.google.common.collect.ImmutableSortedMap.toImmutableSortedMap;
@ -31,6 +32,7 @@ import static google.registry.model.CacheUtils.memoizeWithShortExpiration;
import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.model.registry.Registries.assertTldsExist; import static google.registry.model.registry.Registries.assertTldsExist;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm; import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm;
import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy; import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy;
@ -645,7 +647,20 @@ public class Registrar extends ImmutableObject
} }
private Iterable<RegistrarContact> getContactsIterable() { private Iterable<RegistrarContact> getContactsIterable() {
return ofy().load().type(RegistrarContact.class).ancestor(Registrar.this); if (tm().isOfy()) {
return ofy().load().type(RegistrarContact.class).ancestor(Registrar.this);
} else {
return tm().transact(
() ->
jpaTm()
.getEntityManager()
.createQuery(
"FROM RegistrarPoc WHERE registrarId = :registrarId",
RegistrarContact.class)
.setParameter("registrarId", clientIdentifier)
.getResultStream()
.collect(toImmutableList()));
}
} }
@Override @Override
@ -798,12 +813,12 @@ public class Registrar extends ImmutableObject
* to set the allowed TLDs. * to set the allowed TLDs.
*/ */
public Builder setAllowedTldsUncached(Set<String> allowedTlds) { public Builder setAllowedTldsUncached(Set<String> allowedTlds) {
ImmutableSet<Key<Registry>> newTldKeys = ImmutableSet<VKey<Registry>> newTldKeys =
Sets.difference(allowedTlds, getInstance().getAllowedTlds()).stream() Sets.difference(allowedTlds, getInstance().getAllowedTlds()).stream()
.map(tld -> Key.create(getCrossTldKey(), Registry.class, tld)) .map(Registry::createVKey)
.collect(toImmutableSet()); .collect(toImmutableSet());
Set<Key<Registry>> missingTldKeys = Set<VKey<Registry>> missingTldKeys =
Sets.difference(newTldKeys, ofy().load().keys(newTldKeys).keySet()); Sets.difference(newTldKeys, transactIfJpaTm(() -> tm().load(newTldKeys)).keySet());
checkArgument(missingTldKeys.isEmpty(), "Trying to set nonexisting TLDs: %s", missingTldKeys); checkArgument(missingTldKeys.isEmpty(), "Trying to set nonexisting TLDs: %s", missingTldKeys);
getInstance().allowedTlds = ImmutableSortedSet.copyOf(allowedTlds); getInstance().allowedTlds = ImmutableSortedSet.copyOf(allowedTlds);
return this; return this;
@ -989,7 +1004,9 @@ public class Registrar extends ImmutableObject
/** Loads all registrar entities directly from Datastore. */ /** Loads all registrar entities directly from Datastore. */
public static Iterable<Registrar> loadAll() { public static Iterable<Registrar> loadAll() {
return ImmutableList.copyOf(ofy().load().type(Registrar.class).ancestor(getCrossTldKey())); return tm().isOfy()
? ImmutableList.copyOf(ofy().load().type(Registrar.class).ancestor(getCrossTldKey()))
: tm().transact(() -> tm().loadAll(Registrar.class));
} }
/** Loads all registrar entities using an in-memory cache. */ /** Loads all registrar entities using an in-memory cache. */

View file

@ -38,16 +38,18 @@ import com.google.common.collect.ImmutableSortedSet;
import com.googlecode.objectify.Key; import com.googlecode.objectify.Key;
import google.registry.config.RegistryConfig; import google.registry.config.RegistryConfig;
import google.registry.model.EntityTestCase; import google.registry.model.EntityTestCase;
import google.registry.model.common.EntityGroupRoot;
import google.registry.model.registrar.Registrar.State; import google.registry.model.registrar.Registrar.State;
import google.registry.model.registrar.Registrar.Type; import google.registry.model.registrar.Registrar.Type;
import google.registry.model.registry.Registries; import google.registry.model.registry.Registries;
import google.registry.testing.DualDatabaseTest;
import google.registry.testing.TestOfyAndSql;
import google.registry.testing.TestOfyOnly;
import google.registry.util.CidrAddressBlock; import google.registry.util.CidrAddressBlock;
import org.joda.money.CurrencyUnit; import org.joda.money.CurrencyUnit;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
/** Unit tests for {@link Registrar}. */ /** Unit tests for {@link Registrar}. */
@DualDatabaseTest
class RegistrarTest extends EntityTestCase { class RegistrarTest extends EntityTestCase {
private Registrar registrar; private Registrar registrar;
@ -127,24 +129,17 @@ class RegistrarTest extends EntityTestCase {
.build())); .build()));
} }
@Test @TestOfyAndSql
void testPersistence() { void testPersistence() {
assertThat(registrar) assertThat(tm().transact(() -> tm().load(registrar.createVKey()))).isEqualTo(registrar);
.isEqualTo(
ofy()
.load()
.type(Registrar.class)
.parent(EntityGroupRoot.getCrossTldKey())
.id(registrar.getClientId())
.now());
} }
@Test @TestOfyOnly
void testIndexing() throws Exception { void testIndexing() throws Exception {
verifyIndexing(registrar, "registrarName", "ianaIdentifier"); verifyIndexing(registrar, "registrarName", "ianaIdentifier");
} }
@Test @TestOfyAndSql
void testFailure_passwordNull() { void testFailure_passwordNull() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows( assertThrows(
@ -152,7 +147,7 @@ class RegistrarTest extends EntityTestCase {
assertThat(thrown).hasMessageThat().contains("Password must be 6-16 characters long."); assertThat(thrown).hasMessageThat().contains("Password must be 6-16 characters long.");
} }
@Test @TestOfyAndSql
void testFailure_passwordTooShort() { void testFailure_passwordTooShort() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows( assertThrows(
@ -160,7 +155,7 @@ class RegistrarTest extends EntityTestCase {
assertThat(thrown).hasMessageThat().contains("Password must be 6-16 characters long."); assertThat(thrown).hasMessageThat().contains("Password must be 6-16 characters long.");
} }
@Test @TestOfyAndSql
void testFailure_passwordTooLong() { void testFailure_passwordTooLong() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows( assertThrows(
@ -169,7 +164,7 @@ class RegistrarTest extends EntityTestCase {
assertThat(thrown).hasMessageThat().contains("Password must be 6-16 characters long."); assertThat(thrown).hasMessageThat().contains("Password must be 6-16 characters long.");
} }
@Test @TestOfyAndSql
void testSuccess_clientId_bounds() { void testSuccess_clientId_bounds() {
registrar = registrar.asBuilder().setClientId("abc").build(); registrar = registrar.asBuilder().setClientId("abc").build();
assertThat(registrar.getClientId()).isEqualTo("abc"); assertThat(registrar.getClientId()).isEqualTo("abc");
@ -177,19 +172,19 @@ class RegistrarTest extends EntityTestCase {
assertThat(registrar.getClientId()).isEqualTo("abcdefghijklmnop"); assertThat(registrar.getClientId()).isEqualTo("abcdefghijklmnop");
} }
@Test @TestOfyAndSql
void testFailure_clientId_tooShort() { void testFailure_clientId_tooShort() {
assertThrows(IllegalArgumentException.class, () -> new Registrar.Builder().setClientId("ab")); assertThrows(IllegalArgumentException.class, () -> new Registrar.Builder().setClientId("ab"));
} }
@Test @TestOfyAndSql
void testFailure_clientId_tooLong() { void testFailure_clientId_tooLong() {
assertThrows( assertThrows(
IllegalArgumentException.class, IllegalArgumentException.class,
() -> new Registrar.Builder().setClientId("abcdefghijklmnopq")); () -> new Registrar.Builder().setClientId("abcdefghijklmnopq"));
} }
@Test @TestOfyAndSql
void testSetCertificateHash_alsoSetsHash() { void testSetCertificateHash_alsoSetsHash() {
registrar = registrar.asBuilder().setClientCertificate(null, fakeClock.nowUtc()).build(); registrar = registrar.asBuilder().setClientCertificate(null, fakeClock.nowUtc()).build();
fakeClock.advanceOneMilli(); fakeClock.advanceOneMilli();
@ -199,7 +194,7 @@ class RegistrarTest extends EntityTestCase {
assertThat(registrar.getClientCertificateHash()).isEqualTo(SAMPLE_CERT_HASH); assertThat(registrar.getClientCertificateHash()).isEqualTo(SAMPLE_CERT_HASH);
} }
@Test @TestOfyAndSql
void testDeleteCertificateHash_alsoDeletesHash() { void testDeleteCertificateHash_alsoDeletesHash() {
assertThat(registrar.getClientCertificateHash()).isNotNull(); assertThat(registrar.getClientCertificateHash()).isNotNull();
fakeClock.advanceOneMilli(); fakeClock.advanceOneMilli();
@ -209,7 +204,7 @@ class RegistrarTest extends EntityTestCase {
assertThat(registrar.getClientCertificateHash()).isNull(); assertThat(registrar.getClientCertificateHash()).isNull();
} }
@Test @TestOfyAndSql
void testSetFailoverCertificateHash_alsoSetsHash() { void testSetFailoverCertificateHash_alsoSetsHash() {
fakeClock.advanceOneMilli(); fakeClock.advanceOneMilli();
registrar = registrar =
@ -222,7 +217,7 @@ class RegistrarTest extends EntityTestCase {
assertThat(registrar.getFailoverClientCertificateHash()).isEqualTo(SAMPLE_CERT2_HASH); assertThat(registrar.getFailoverClientCertificateHash()).isEqualTo(SAMPLE_CERT2_HASH);
} }
@Test @TestOfyAndSql
void testDeleteFailoverCertificateHash_alsoDeletesHash() { void testDeleteFailoverCertificateHash_alsoDeletesHash() {
registrar = registrar =
registrar.asBuilder().setFailoverClientCertificate(SAMPLE_CERT, fakeClock.nowUtc()).build(); registrar.asBuilder().setFailoverClientCertificate(SAMPLE_CERT, fakeClock.nowUtc()).build();
@ -235,7 +230,7 @@ class RegistrarTest extends EntityTestCase {
assertThat(registrar.getFailoverClientCertificateHash()).isNull(); assertThat(registrar.getFailoverClientCertificateHash()).isNull();
} }
@Test @TestOfyAndSql
void testSuccess_clearingIanaAndBillingIds() { void testSuccess_clearingIanaAndBillingIds() {
registrar registrar
.asBuilder() .asBuilder()
@ -245,30 +240,30 @@ class RegistrarTest extends EntityTestCase {
.build(); .build();
} }
@Test @TestOfyAndSql
void testSuccess_clearingBillingAccountMap() { void testSuccess_clearingBillingAccountMap() {
registrar = registrar.asBuilder().setBillingAccountMap(null).build(); registrar = registrar.asBuilder().setBillingAccountMap(null).build();
assertThat(registrar.getBillingAccountMap()).isEmpty(); assertThat(registrar.getBillingAccountMap()).isEmpty();
} }
@Test @TestOfyAndSql
void testSuccess_ianaIdForInternal() { void testSuccess_ianaIdForInternal() {
registrar.asBuilder().setType(Type.INTERNAL).setIanaIdentifier(9998L).build(); registrar.asBuilder().setType(Type.INTERNAL).setIanaIdentifier(9998L).build();
registrar.asBuilder().setType(Type.INTERNAL).setIanaIdentifier(9999L).build(); registrar.asBuilder().setType(Type.INTERNAL).setIanaIdentifier(9999L).build();
} }
@Test @TestOfyAndSql
void testSuccess_ianaIdForPdt() { void testSuccess_ianaIdForPdt() {
registrar.asBuilder().setType(Type.PDT).setIanaIdentifier(9995L).build(); registrar.asBuilder().setType(Type.PDT).setIanaIdentifier(9995L).build();
registrar.asBuilder().setType(Type.PDT).setIanaIdentifier(9996L).build(); registrar.asBuilder().setType(Type.PDT).setIanaIdentifier(9996L).build();
} }
@Test @TestOfyAndSql
void testSuccess_ianaIdForExternalMonitoring() { void testSuccess_ianaIdForExternalMonitoring() {
registrar.asBuilder().setType(Type.EXTERNAL_MONITORING).setIanaIdentifier(9997L).build(); registrar.asBuilder().setType(Type.EXTERNAL_MONITORING).setIanaIdentifier(9997L).build();
} }
@Test @TestOfyAndSql
void testSuccess_emptyContactTypesAllowed() { void testSuccess_emptyContactTypesAllowed() {
persistSimpleResource( persistSimpleResource(
new RegistrarContact.Builder() new RegistrarContact.Builder()
@ -284,7 +279,7 @@ class RegistrarTest extends EntityTestCase {
} }
} }
@Test @TestOfyAndSql
void testSuccess_getContactsByType() { void testSuccess_getContactsByType() {
RegistrarContact newTechContact = RegistrarContact newTechContact =
persistSimpleResource( persistSimpleResource(
@ -318,7 +313,7 @@ class RegistrarTest extends EntityTestCase {
assertThat(abuseContacts).containsExactly(newTechAbuseContact, abuseAdminContact).inOrder(); assertThat(abuseContacts).containsExactly(newTechAbuseContact, abuseAdminContact).inOrder();
} }
@Test @TestOfyAndSql
void testFailure_missingRegistrarType() { void testFailure_missingRegistrarType() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows( assertThrows(
@ -327,7 +322,7 @@ class RegistrarTest extends EntityTestCase {
assertThat(thrown).hasMessageThat().contains("Registrar type cannot be null"); assertThat(thrown).hasMessageThat().contains("Registrar type cannot be null");
} }
@Test @TestOfyAndSql
void testFailure_missingRegistrarName() { void testFailure_missingRegistrarName() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows( assertThrows(
@ -337,7 +332,7 @@ class RegistrarTest extends EntityTestCase {
assertThat(thrown).hasMessageThat().contains("Registrar name cannot be null"); assertThat(thrown).hasMessageThat().contains("Registrar name cannot be null");
} }
@Test @TestOfyAndSql
void testFailure_missingAddress() { void testFailure_missingAddress() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows( assertThrows(
@ -353,21 +348,21 @@ class RegistrarTest extends EntityTestCase {
.contains("Must specify at least one of localized or internationalized address"); .contains("Must specify at least one of localized or internationalized address");
} }
@Test @TestOfyAndSql
void testFailure_badIanaIdForInternal() { void testFailure_badIanaIdForInternal() {
assertThrows( assertThrows(
IllegalArgumentException.class, IllegalArgumentException.class,
() -> new Registrar.Builder().setType(Type.INTERNAL).setIanaIdentifier(8L).build()); () -> new Registrar.Builder().setType(Type.INTERNAL).setIanaIdentifier(8L).build());
} }
@Test @TestOfyAndSql
void testFailure_badIanaIdForPdt() { void testFailure_badIanaIdForPdt() {
assertThrows( assertThrows(
IllegalArgumentException.class, IllegalArgumentException.class,
() -> new Registrar.Builder().setType(Type.PDT).setIanaIdentifier(8L).build()); () -> new Registrar.Builder().setType(Type.PDT).setIanaIdentifier(8L).build());
} }
@Test @TestOfyAndSql
void testFailure_badIanaIdForExternalMonitoring() { void testFailure_badIanaIdForExternalMonitoring() {
assertThrows( assertThrows(
IllegalArgumentException.class, IllegalArgumentException.class,
@ -375,51 +370,51 @@ class RegistrarTest extends EntityTestCase {
registrar.asBuilder().setType(Type.EXTERNAL_MONITORING).setIanaIdentifier(8L).build()); registrar.asBuilder().setType(Type.EXTERNAL_MONITORING).setIanaIdentifier(8L).build());
} }
@Test @TestOfyAndSql
void testFailure_missingIanaIdForReal() { void testFailure_missingIanaIdForReal() {
assertThrows( assertThrows(
IllegalArgumentException.class, () -> new Registrar.Builder().setType(Type.REAL).build()); IllegalArgumentException.class, () -> new Registrar.Builder().setType(Type.REAL).build());
} }
@Test @TestOfyAndSql
void testFailure_missingIanaIdForInternal() { void testFailure_missingIanaIdForInternal() {
assertThrows( assertThrows(
IllegalArgumentException.class, IllegalArgumentException.class,
() -> new Registrar.Builder().setType(Type.INTERNAL).build()); () -> new Registrar.Builder().setType(Type.INTERNAL).build());
} }
@Test @TestOfyAndSql
void testFailure_missingIanaIdForPdt() { void testFailure_missingIanaIdForPdt() {
assertThrows( assertThrows(
IllegalArgumentException.class, () -> new Registrar.Builder().setType(Type.PDT).build()); IllegalArgumentException.class, () -> new Registrar.Builder().setType(Type.PDT).build());
} }
@Test @TestOfyAndSql
void testFailure_missingIanaIdForExternalMonitoring() { void testFailure_missingIanaIdForExternalMonitoring() {
assertThrows( assertThrows(
IllegalArgumentException.class, IllegalArgumentException.class,
() -> new Registrar.Builder().setType(Type.EXTERNAL_MONITORING).build()); () -> new Registrar.Builder().setType(Type.EXTERNAL_MONITORING).build());
} }
@Test @TestOfyAndSql
void testFailure_phonePasscodeTooShort() { void testFailure_phonePasscodeTooShort() {
assertThrows( assertThrows(
IllegalArgumentException.class, () -> new Registrar.Builder().setPhonePasscode("0123")); IllegalArgumentException.class, () -> new Registrar.Builder().setPhonePasscode("0123"));
} }
@Test @TestOfyAndSql
void testFailure_phonePasscodeTooLong() { void testFailure_phonePasscodeTooLong() {
assertThrows( assertThrows(
IllegalArgumentException.class, () -> new Registrar.Builder().setPhonePasscode("012345")); IllegalArgumentException.class, () -> new Registrar.Builder().setPhonePasscode("012345"));
} }
@Test @TestOfyAndSql
void testFailure_phonePasscodeInvalidCharacters() { void testFailure_phonePasscodeInvalidCharacters() {
assertThrows( assertThrows(
IllegalArgumentException.class, () -> new Registrar.Builder().setPhonePasscode("code1")); IllegalArgumentException.class, () -> new Registrar.Builder().setPhonePasscode("code1"));
} }
@Test @TestOfyAndSql
void testSuccess_setAllowedTlds() { void testSuccess_setAllowedTlds() {
assertThat( assertThat(
registrar registrar
@ -430,7 +425,7 @@ class RegistrarTest extends EntityTestCase {
.containsExactly("xn--q9jyb4c"); .containsExactly("xn--q9jyb4c");
} }
@Test @TestOfyAndSql
void testSuccess_setAllowedTldsUncached() { void testSuccess_setAllowedTldsUncached() {
assertThat( assertThat(
registrar registrar
@ -441,21 +436,21 @@ class RegistrarTest extends EntityTestCase {
.containsExactly("xn--q9jyb4c"); .containsExactly("xn--q9jyb4c");
} }
@Test @TestOfyAndSql
void testFailure_setAllowedTlds_nonexistentTld() { void testFailure_setAllowedTlds_nonexistentTld() {
assertThrows( assertThrows(
IllegalArgumentException.class, IllegalArgumentException.class,
() -> registrar.asBuilder().setAllowedTlds(ImmutableSet.of("bad"))); () -> registrar.asBuilder().setAllowedTlds(ImmutableSet.of("bad")));
} }
@Test @TestOfyAndSql
void testFailure_setAllowedTldsUncached_nonexistentTld() { void testFailure_setAllowedTldsUncached_nonexistentTld() {
assertThrows( assertThrows(
IllegalArgumentException.class, IllegalArgumentException.class,
() -> registrar.asBuilder().setAllowedTldsUncached(ImmutableSet.of("bad"))); () -> registrar.asBuilder().setAllowedTldsUncached(ImmutableSet.of("bad")));
} }
@Test @TestOfyAndSql
void testFailure_driveFolderId_asFullUrl() { void testFailure_driveFolderId_asFullUrl() {
String driveFolderId = String driveFolderId =
"https://drive.google.com/drive/folders/1j3v7RZkU25DjbTx2-Q93H04zKOBau89M"; "https://drive.google.com/drive/folders/1j3v7RZkU25DjbTx2-Q93H04zKOBau89M";
@ -466,14 +461,14 @@ class RegistrarTest extends EntityTestCase {
assertThat(thrown).hasMessageThat().isEqualTo("Drive folder ID must not be a full URL"); assertThat(thrown).hasMessageThat().isEqualTo("Drive folder ID must not be a full URL");
} }
@Test @TestOfyAndSql
void testFailure_nullEmail() { void testFailure_nullEmail() {
NullPointerException thrown = NullPointerException thrown =
assertThrows(NullPointerException.class, () -> registrar.asBuilder().setEmailAddress(null)); assertThrows(NullPointerException.class, () -> registrar.asBuilder().setEmailAddress(null));
assertThat(thrown).hasMessageThat().isEqualTo("Provided email was null"); assertThat(thrown).hasMessageThat().isEqualTo("Provided email was null");
} }
@Test @TestOfyAndSql
void testFailure_invalidEmail() { void testFailure_invalidEmail() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows( assertThrows(
@ -483,7 +478,7 @@ class RegistrarTest extends EntityTestCase {
.isEqualTo("Provided email lolcat is not a valid email address"); .isEqualTo("Provided email lolcat is not a valid email address");
} }
@Test @TestOfyAndSql
void testFailure_emptyEmail() { void testFailure_emptyEmail() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows( assertThrows(
@ -491,7 +486,7 @@ class RegistrarTest extends EntityTestCase {
assertThat(thrown).hasMessageThat().isEqualTo("Provided email is not a valid email address"); assertThat(thrown).hasMessageThat().isEqualTo("Provided email is not a valid email address");
} }
@Test @TestOfyAndSql
void testFailure_nullIcannReferralEmail() { void testFailure_nullIcannReferralEmail() {
NullPointerException thrown = NullPointerException thrown =
assertThrows( assertThrows(
@ -499,7 +494,7 @@ class RegistrarTest extends EntityTestCase {
assertThat(thrown).hasMessageThat().isEqualTo("Provided email was null"); assertThat(thrown).hasMessageThat().isEqualTo("Provided email was null");
} }
@Test @TestOfyAndSql
void testFailure_invalidIcannReferralEmail() { void testFailure_invalidIcannReferralEmail() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows( assertThrows(
@ -510,7 +505,7 @@ class RegistrarTest extends EntityTestCase {
.isEqualTo("Provided email lolcat is not a valid email address"); .isEqualTo("Provided email lolcat is not a valid email address");
} }
@Test @TestOfyAndSql
void testFailure_emptyIcannReferralEmail() { void testFailure_emptyIcannReferralEmail() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows( assertThrows(
@ -518,7 +513,7 @@ class RegistrarTest extends EntityTestCase {
assertThat(thrown).hasMessageThat().isEqualTo("Provided email is not a valid email address"); assertThat(thrown).hasMessageThat().isEqualTo("Provided email is not a valid email address");
} }
@Test @TestOfyAndSql
void testSuccess_setAllowedTldsUncached_newTldNotInCache() { void testSuccess_setAllowedTldsUncached_newTldNotInCache() {
int origSingletonCacheRefreshSeconds = int origSingletonCacheRefreshSeconds =
RegistryConfig.CONFIG_SETTINGS.get().caching.singletonCacheRefreshSeconds; RegistryConfig.CONFIG_SETTINGS.get().caching.singletonCacheRefreshSeconds;
@ -567,45 +562,45 @@ class RegistrarTest extends EntityTestCase {
} }
} }
@Test @TestOfyOnly
void testLoadByClientIdCached_isTransactionless() { void testLoadByClientIdCached_isTransactionless() {
tm().transact( tm().transact(
() -> { () -> {
assertThat(Registrar.loadByClientIdCached("registrar")).isPresent(); assertThat(Registrar.loadByClientIdCached("registrar")).isPresent();
// Load something as a control to make sure we are seeing loaded keys in the session // Load something as a control to make sure we are seeing loaded keys in the
// cache. // session cache.
ofy().load().entity(abuseAdminContact).now(); ofy().load().entity(abuseAdminContact).now();
assertThat(ofy().getSessionKeys()).contains(Key.create(abuseAdminContact)); assertThat(ofy().getSessionKeys()).contains(Key.create(abuseAdminContact));
assertThat(ofy().getSessionKeys()).doesNotContain(Key.create(registrar)); assertThat(ofy().getSessionKeys()).doesNotContain(Key.create(registrar));
}); });
ofy().clearSessionCache(); tm().clearSessionCache();
// Conversely, loads outside of a transaction should end up in the session cache. // Conversely, loads outside of a transaction should end up in the session cache.
assertThat(Registrar.loadByClientIdCached("registrar")).isPresent(); assertThat(Registrar.loadByClientIdCached("registrar")).isPresent();
assertThat(ofy().getSessionKeys()).contains(Key.create(registrar)); assertThat(ofy().getSessionKeys()).contains(Key.create(registrar));
} }
@Test @TestOfyAndSql
void testFailure_loadByClientId_clientIdIsNull() { void testFailure_loadByClientId_clientIdIsNull() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows(IllegalArgumentException.class, () -> Registrar.loadByClientId(null)); assertThrows(IllegalArgumentException.class, () -> Registrar.loadByClientId(null));
assertThat(thrown).hasMessageThat().contains("clientId must be specified"); assertThat(thrown).hasMessageThat().contains("clientId must be specified");
} }
@Test @TestOfyAndSql
void testFailure_loadByClientId_clientIdIsEmpty() { void testFailure_loadByClientId_clientIdIsEmpty() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows(IllegalArgumentException.class, () -> Registrar.loadByClientId("")); assertThrows(IllegalArgumentException.class, () -> Registrar.loadByClientId(""));
assertThat(thrown).hasMessageThat().contains("clientId must be specified"); assertThat(thrown).hasMessageThat().contains("clientId must be specified");
} }
@Test @TestOfyAndSql
void testFailure_loadByClientIdCached_clientIdIsNull() { void testFailure_loadByClientIdCached_clientIdIsNull() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows(IllegalArgumentException.class, () -> Registrar.loadByClientIdCached(null)); assertThrows(IllegalArgumentException.class, () -> Registrar.loadByClientIdCached(null));
assertThat(thrown).hasMessageThat().contains("clientId must be specified"); assertThat(thrown).hasMessageThat().contains("clientId must be specified");
} }
@Test @TestOfyAndSql
void testFailure_loadByClientIdCached_clientIdIsEmpty() { void testFailure_loadByClientIdCached_clientIdIsEmpty() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows(IllegalArgumentException.class, () -> Registrar.loadByClientIdCached("")); assertThrows(IllegalArgumentException.class, () -> Registrar.loadByClientIdCached(""));

View file

@ -1144,13 +1144,15 @@ public class DatastoreHelper {
/** Force the create and update timestamps to get written into the resource. **/ /** Force the create and update timestamps to get written into the resource. **/
public static <R> R cloneAndSetAutoTimestamps(final R resource) { public static <R> R cloneAndSetAutoTimestamps(final R resource) {
return tm().transact( if (tm().isOfy()) {
tm().isOfy() return tm().transact(() -> ofy().load().fromEntity(ofy().save().toEntity(resource)));
? () -> ofy().load().fromEntity(ofy().save().toEntity(resource)) } else {
: () -> { // We have to separate the read and write operation into different transactions
tm().put(resource); // otherwise JPA would just return the input entity instead of actually creating a
return tm().load(resource); // clone.
}); tm().transact(() -> tm().put(resource));
return tm().transact(() -> tm().load(resource));
}
} }
/** Returns the entire map of {@link PremiumListEntry}s for the given {@link PremiumList}. */ /** Returns the entire map of {@link PremiumListEntry}s for the given {@link PremiumList}. */