From bc091f25ca1bf888881f33564eabcbcaa34b9e7b Mon Sep 17 00:00:00 2001 From: Lai Jiang Date: Wed, 7 Sep 2022 14:24:42 -0400 Subject: [PATCH] Remove ofy support from registrar (#1762) Also fixes some warnings about the use of raw types. --- .../google/registry/model/EntityClasses.java | 2 - .../registry/model/ImmutableObject.java | 2 +- .../registry/model/billing/BillingEvent.java | 3 + .../registry/model/poll/PollMessage.java | 3 + .../registry/model/registrar/Registrar.java | 85 ++++++------------- .../registry/persistence/EppHistoryVKey.java | 4 +- .../model/common/ClassPathManagerTest.java | 3 - .../registry/tools/MutatingCommandTest.java | 27 +++--- .../registry/tools/SetupOteCommandTest.java | 2 +- .../google/registry/model/schema.txt | 48 ----------- 10 files changed, 48 insertions(+), 131 deletions(-) diff --git a/core/src/main/java/google/registry/model/EntityClasses.java b/core/src/main/java/google/registry/model/EntityClasses.java index 90fb5c0b8..33ba9fc7e 100644 --- a/core/src/main/java/google/registry/model/EntityClasses.java +++ b/core/src/main/java/google/registry/model/EntityClasses.java @@ -27,7 +27,6 @@ import google.registry.model.host.HostHistory; import google.registry.model.index.EppResourceIndex; import google.registry.model.index.EppResourceIndexBucket; import google.registry.model.index.ForeignKeyIndex; -import google.registry.model.registrar.Registrar; import google.registry.model.reporting.HistoryEntry; import google.registry.model.server.Lock; import google.registry.model.server.ServerSecret; @@ -54,7 +53,6 @@ public final class EntityClasses { Host.class, HostHistory.class, Lock.class, - Registrar.class, ServerSecret.class); private EntityClasses() {} diff --git a/core/src/main/java/google/registry/model/ImmutableObject.java b/core/src/main/java/google/registry/model/ImmutableObject.java index db4ec2ec1..66b78cef0 100644 --- a/core/src/main/java/google/registry/model/ImmutableObject.java +++ b/core/src/main/java/google/registry/model/ImmutableObject.java @@ -257,7 +257,7 @@ public abstract class ImmutableObject implements Cloneable { return (Map) toMapRecursive(this); } - public VKey createVKey() { + public VKey createVKey() { throw new UnsupportedOperationException("VKey creation is not supported for this entity"); } } diff --git a/core/src/main/java/google/registry/model/billing/BillingEvent.java b/core/src/main/java/google/registry/model/billing/BillingEvent.java index 907ad6368..ca1de417c 100644 --- a/core/src/main/java/google/registry/model/billing/BillingEvent.java +++ b/core/src/main/java/google/registry/model/billing/BillingEvent.java @@ -212,6 +212,9 @@ public abstract class BillingEvent extends ImmutableObject return nullToEmptyImmutableCopy(flags); } + @Override + public abstract VKey createVKey(); + /** Override Buildable.asBuilder() to give this method stronger typing. */ @Override public abstract Builder asBuilder(); diff --git a/core/src/main/java/google/registry/model/poll/PollMessage.java b/core/src/main/java/google/registry/model/poll/PollMessage.java index af4800c27..b4052c964 100644 --- a/core/src/main/java/google/registry/model/poll/PollMessage.java +++ b/core/src/main/java/google/registry/model/poll/PollMessage.java @@ -215,6 +215,9 @@ public abstract class PollMessage extends ImmutableObject return domainRepoId != null ? Type.DOMAIN : contactRepoId != null ? Type.CONTACT : Type.HOST; } + @Override + public abstract VKey createVKey(); + public abstract ImmutableList getResponseData(); /** Override Buildable.asBuilder() to give this method stronger typing. */ diff --git a/core/src/main/java/google/registry/model/registrar/Registrar.java b/core/src/main/java/google/registry/model/registrar/Registrar.java index c01f9efd6..0a811dbec 100644 --- a/core/src/main/java/google/registry/model/registrar/Registrar.java +++ b/core/src/main/java/google/registry/model/registrar/Registrar.java @@ -26,8 +26,6 @@ import static com.google.common.collect.Sets.immutableEnumSet; import static com.google.common.io.BaseEncoding.base64; import static google.registry.config.RegistryConfig.getDefaultRegistrarWhoisServer; import static google.registry.model.CacheUtils.memoizeWithShortExpiration; -import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; -import static google.registry.model.ofy.ObjectifyService.auditedOfy; import static google.registry.model.tld.Registries.assertTldsExist; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; @@ -44,7 +42,6 @@ import static java.util.function.Predicate.isEqual; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; -import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -55,12 +52,6 @@ import com.google.common.collect.Range; import com.google.common.collect.Sets; import com.google.common.collect.Streams; import com.google.re2j.Pattern; -import com.googlecode.objectify.Key; -import com.googlecode.objectify.annotation.Entity; -import com.googlecode.objectify.annotation.Id; -import com.googlecode.objectify.annotation.Ignore; -import com.googlecode.objectify.annotation.Index; -import com.googlecode.objectify.annotation.Parent; import google.registry.model.Buildable; import google.registry.model.CreateAutoTimestamp; import google.registry.model.ImmutableObject; @@ -68,9 +59,6 @@ import google.registry.model.JsonMapBuilder; import google.registry.model.Jsonifiable; import google.registry.model.UnsafeSerializable; import google.registry.model.UpdateAutoTimestamp; -import google.registry.model.annotations.InCrossTld; -import google.registry.model.annotations.ReportedOn; -import google.registry.model.common.EntityGroupRoot; import google.registry.model.tld.Registry; import google.registry.model.tld.Registry.TldType; import google.registry.persistence.VKey; @@ -83,6 +71,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.function.Predicate; +import java.util.function.Supplier; import javax.annotation.Nullable; import javax.mail.internet.AddressException; import javax.mail.internet.InternetAddress; @@ -90,25 +79,22 @@ import javax.persistence.AttributeOverride; import javax.persistence.AttributeOverrides; import javax.persistence.Column; import javax.persistence.Embedded; +import javax.persistence.Entity; import javax.persistence.EnumType; import javax.persistence.Enumerated; +import javax.persistence.Id; +import javax.persistence.Index; import javax.persistence.Table; -import javax.persistence.Transient; import org.joda.money.CurrencyUnit; import org.joda.time.DateTime; /** Information about a registrar. */ -@ReportedOn @Entity -@javax.persistence.Entity @Table( indexes = { - @javax.persistence.Index(columnList = "registrarName", name = "registrar_name_idx"), - @javax.persistence.Index( - columnList = "ianaIdentifier", - name = "registrar_iana_identifier_idx"), + @Index(columnList = "registrarName", name = "registrar_name_idx"), + @Index(columnList = "ianaIdentifier", name = "registrar_iana_identifier_idx"), }) -@InCrossTld public class Registrar extends ImmutableObject implements Buildable, Jsonifiable, UnsafeSerializable { @@ -197,7 +183,7 @@ public class Registrar extends ImmutableObject /** The states in which a {@link Registrar} is considered {@link #isLive live}. */ private static final ImmutableSet LIVE_STATES = - Sets.immutableEnumSet(State.ACTIVE, State.SUSPENDED); + immutableEnumSet(State.ACTIVE, State.SUSPENDED); /** * The types for which a {@link Registrar} should be included in WHOIS and RDAP output. We exclude @@ -213,18 +199,9 @@ public class Registrar extends ImmutableObject private static final Comparator CONTACT_EMAIL_COMPARATOR = comparing(RegistrarPoc::getEmailAddress, String::compareTo); - /** - * A caching {@link Supplier} of a registrarId to {@link Registrar} map. - * - *

The supplier's get() method enters a transactionless context briefly to avoid enrolling the - * query inside an unrelated client-affecting transaction. - */ + /** A caching {@link Supplier} of a registrarId to {@link Registrar} map. */ private static final Supplier> CACHE_BY_REGISTRAR_ID = - memoizeWithShortExpiration( - () -> - tm().doTransactionless(() -> Maps.uniqueIndex(loadAll(), Registrar::getRegistrarId))); - - @Parent @Transient Key parent = getCrossTldKey(); + memoizeWithShortExpiration(() -> Maps.uniqueIndex(loadAll(), Registrar::getRegistrarId)); /** * Unique registrar client id. Must conform to "clIDType" as defined in RFC5730. @@ -233,7 +210,6 @@ public class Registrar extends ImmutableObject *

TODO(b/177567432): Rename this field to registrarId. */ @Id - @javax.persistence.Id @Column(name = "registrarId", nullable = false) String clientIdentifier; @@ -248,7 +224,6 @@ public class Registrar extends ImmutableObject * @see ICANN-Accredited * Registrars */ - @Index @Column(nullable = false) String registrarName; @@ -313,7 +288,6 @@ public class Registrar extends ImmutableObject * Localized {@link RegistrarAddress} for this registrar. Contents can be represented in * unrestricted UTF-8. */ - @Ignore @Embedded @AttributeOverrides({ @AttributeOverride( @@ -338,7 +312,6 @@ public class Registrar extends ImmutableObject * Internationalized {@link RegistrarAddress} for this registrar. All contained values must be * representable in the 7-bit US-ASCII character set. */ - @Ignore @Embedded @AttributeOverrides({ @AttributeOverride(name = "streetLine1", column = @Column(name = "i18n_address_street_line1")), @@ -367,14 +340,14 @@ public class Registrar extends ImmutableObject * *

    *
  • 8 is used for Testing Registrar. - *
  • 9997 is used by ICAAN for SLA monitoring. + *
  • 9997 is used by ICANN for SLA monitoring. *
  • 9999 is used for cases when the registry operator acts as registrar. *
* * @see Registrar * IDs */ - @Index @Nullable Long ianaIdentifier; + @Nullable Long ianaIdentifier; /** Purchase Order number used for invoices in external billing system, if applicable. */ @Nullable String poNumber; @@ -395,7 +368,7 @@ public class Registrar extends ImmutableObject /** * ICANN referral email address. * - *

This value is specified in the initial registrar contact. It can't be edited in the web GUI + *

This value is specified in the initial registrar contact. It can't be edited in the web GUI, * and it must be specified when the registrar account is created. */ String icannReferralEmail; @@ -406,10 +379,10 @@ public class Registrar extends ImmutableObject // Metadata. /** The time when this registrar was created. */ - @Ignore CreateAutoTimestamp creationTime = CreateAutoTimestamp.create(null); + CreateAutoTimestamp creationTime = CreateAutoTimestamp.create(null); /** An automatically managed last-saved timestamp. */ - @Ignore UpdateAutoTimestamp lastUpdateTime = UpdateAutoTimestamp.create(null); + UpdateAutoTimestamp lastUpdateTime = UpdateAutoTimestamp.create(null); /** The time that the certificate was last updated. */ DateTime lastCertificateUpdateTime; @@ -610,18 +583,13 @@ public class Registrar extends ImmutableObject } private Iterable getContactsIterable() { - if (tm().isOfy()) { - return auditedOfy().load().type(RegistrarPoc.class).ancestor(Registrar.this); - } else { - return tm().transact( - () -> - jpaTm() - .query( - "FROM RegistrarPoc WHERE registrarId = :registrarId", RegistrarPoc.class) - .setParameter("registrarId", clientIdentifier) - .getResultStream() - .collect(toImmutableList())); - } + return tm().transact( + () -> + jpaTm() + .query("FROM RegistrarPoc WHERE registrarId = :registrarId", RegistrarPoc.class) + .setParameter("registrarId", clientIdentifier) + .getResultStream() + .collect(toImmutableList())); } @Override @@ -687,18 +655,13 @@ public class Registrar extends ImmutableObject /** Creates a {@link VKey} for this instance. */ @Override public VKey createVKey() { - return createVKey(Key.create(this)); + return createVKey(clientIdentifier); } /** Creates a {@link VKey} for the given {@code registrarId}. */ public static VKey createVKey(String registrarId) { checkArgumentNotNull(registrarId, "registrarId must be specified"); - return createVKey(Key.create(getCrossTldKey(), Registrar.class, registrarId)); - } - - /** Creates a {@link VKey} instance from a {@link Key} instance. */ - public static VKey createVKey(Key key) { - return VKey.create(Registrar.class, key.getName(), key); + return VKey.createSql(Registrar.class, registrarId); } /** A builder for constructing {@link Registrar}, since it is immutable. */ @@ -774,7 +737,7 @@ public class Registrar extends ImmutableObject Set> missingTldKeys = Sets.difference( newTldKeys, tm().transact(() -> tm().loadByKeysIfPresent(newTldKeys)).keySet()); - checkArgument(missingTldKeys.isEmpty(), "Trying to set nonexisting TLDs: %s", missingTldKeys); + checkArgument(missingTldKeys.isEmpty(), "Trying to set nonexistent TLDs: %s", missingTldKeys); getInstance().allowedTlds = ImmutableSortedSet.copyOf(allowedTlds); return this; } diff --git a/core/src/main/java/google/registry/persistence/EppHistoryVKey.java b/core/src/main/java/google/registry/persistence/EppHistoryVKey.java index 8adeea2be..d9e683c47 100644 --- a/core/src/main/java/google/registry/persistence/EppHistoryVKey.java +++ b/core/src/main/java/google/registry/persistence/EppHistoryVKey.java @@ -38,8 +38,8 @@ import javax.persistence.MappedSuperclass; */ @MappedSuperclass @Access(AccessType.FIELD) -public abstract class EppHistoryVKey extends ImmutableObject - implements Serializable { +public abstract class EppHistoryVKey + extends ImmutableObject implements Serializable { private static final long serialVersionUID = -3906580677709539818L; diff --git a/core/src/test/java/google/registry/model/common/ClassPathManagerTest.java b/core/src/test/java/google/registry/model/common/ClassPathManagerTest.java index 659bc1857..179e56ea4 100644 --- a/core/src/test/java/google/registry/model/common/ClassPathManagerTest.java +++ b/core/src/test/java/google/registry/model/common/ClassPathManagerTest.java @@ -26,7 +26,6 @@ import google.registry.model.index.EppResourceIndexBucket; import google.registry.model.index.ForeignKeyIndex.ForeignKeyContactIndex; import google.registry.model.index.ForeignKeyIndex.ForeignKeyDomainIndex; import google.registry.model.index.ForeignKeyIndex.ForeignKeyHostIndex; -import google.registry.model.registrar.Registrar; import google.registry.model.reporting.HistoryEntry; import google.registry.model.server.Lock; import google.registry.model.server.ServerSecret; @@ -49,7 +48,6 @@ public class ClassPathManagerTest { assertThat(ClassPathManager.getClass("ForeignKeyContactIndex")) .isEqualTo(ForeignKeyContactIndex.class); assertThat(ClassPathManager.getClass("Host")).isEqualTo(Host.class); - assertThat(ClassPathManager.getClass("Registrar")).isEqualTo(Registrar.class); assertThat(ClassPathManager.getClass("Contact")).isEqualTo(Contact.class); assertThat(ClassPathManager.getClass("GaeUserIdConverter")).isEqualTo(GaeUserIdConverter.class); assertThat(ClassPathManager.getClass("EppResourceIndexBucket")) @@ -99,7 +97,6 @@ public class ClassPathManagerTest { assertThat(ClassPathManager.getClassName(ForeignKeyContactIndex.class)) .isEqualTo("ForeignKeyContactIndex"); assertThat(ClassPathManager.getClassName(Host.class)).isEqualTo("Host"); - assertThat(ClassPathManager.getClassName(Registrar.class)).isEqualTo("Registrar"); assertThat(ClassPathManager.getClassName(Contact.class)).isEqualTo("Contact"); assertThat(ClassPathManager.getClassName(GaeUserIdConverter.class)) .isEqualTo("GaeUserIdConverter"); diff --git a/core/src/test/java/google/registry/tools/MutatingCommandTest.java b/core/src/test/java/google/registry/tools/MutatingCommandTest.java index c8f66063f..3fd812ac9 100644 --- a/core/src/test/java/google/registry/tools/MutatingCommandTest.java +++ b/core/src/test/java/google/registry/tools/MutatingCommandTest.java @@ -28,13 +28,13 @@ import static org.joda.time.DateTimeZone.UTC; import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableList; -import com.googlecode.objectify.Key; +import google.registry.model.ImmutableObject; import google.registry.model.host.Host; import google.registry.model.registrar.Registrar; import google.registry.persistence.VKey; import google.registry.testing.AppEngineExtension; -import java.util.Arrays; import java.util.Optional; +import java.util.stream.Stream; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -119,19 +119,20 @@ public class MutatingCommandTest { @Test void testSuccess_create() throws Exception { ImmutableList> keys = - Arrays.asList(host1, host2, registrar1, registrar2).stream() - .map(entity -> VKey.from(Key.create(entity))) + Stream.of(host1, host2, registrar1, registrar2) + .map(ImmutableObject::createVKey) .collect(toImmutableList()); tm().transact(() -> tm().delete(keys)); - MutatingCommand command = new MutatingCommand() { - @Override - protected void init() { - stageEntityChange(null, newHost1); - stageEntityChange(null, newHost2); - stageEntityChange(null, newRegistrar1); - stageEntityChange(null, newRegistrar2); - } - }; + MutatingCommand command = + new MutatingCommand() { + @Override + protected void init() { + stageEntityChange(null, newHost1); + stageEntityChange(null, newHost2); + stageEntityChange(null, newRegistrar1); + stageEntityChange(null, newRegistrar2); + } + }; command.init(); String changes = command.prompt(); assertThat(changes) diff --git a/core/src/test/java/google/registry/tools/SetupOteCommandTest.java b/core/src/test/java/google/registry/tools/SetupOteCommandTest.java index f69a0f56b..8e609949a 100644 --- a/core/src/test/java/google/registry/tools/SetupOteCommandTest.java +++ b/core/src/test/java/google/registry/tools/SetupOteCommandTest.java @@ -366,7 +366,7 @@ class SetupOteCommandTest extends CommandTestCase { "--registrar=blobio", "--email=contact@email.com", "--certfile=" + getCertFilename())); - assertThat(thrown).hasMessageThat().contains("VKey(sql:blobio-1,ofy:blobio-1)"); + assertThat(thrown).hasMessageThat().contains("VKey(sql:blobio-1)"); } @Test diff --git a/core/src/test/resources/google/registry/model/schema.txt b/core/src/test/resources/google/registry/model/schema.txt index 2743a1437..e91f1a39d 100644 --- a/core/src/test/resources/google/registry/model/schema.txt +++ b/core/src/test/resources/google/registry/model/schema.txt @@ -312,54 +312,6 @@ class google.registry.model.index.ForeignKeyIndex$ForeignKeyHostIndex { google.registry.persistence.VKey topReference; org.joda.time.DateTime deletionTime; } -class google.registry.model.registrar.Registrar { - @Id java.lang.String clientIdentifier; - @Parent com.googlecode.objectify.Key parent; - boolean blockPremiumNames; - boolean contactsRequireSyncing; - boolean registryLockAllowed; - google.registry.model.registrar.Registrar$State state; - google.registry.model.registrar.Registrar$Type type; - java.lang.Long ianaIdentifier; - java.lang.String clientCertificate; - java.lang.String clientCertificateHash; - java.lang.String driveFolderId; - java.lang.String emailAddress; - java.lang.String failoverClientCertificate; - java.lang.String failoverClientCertificateHash; - java.lang.String faxNumber; - java.lang.String icannReferralEmail; - java.lang.String passwordHash; - java.lang.String phoneNumber; - java.lang.String phonePasscode; - java.lang.String poNumber; - java.lang.String registrarName; - java.lang.String salt; - java.lang.String url; - java.lang.String whoisServer; - java.util.List ipAddressWhitelist; - java.util.Map billingAccountMap; - java.util.Set allowedTlds; - java.util.Set rdapBaseUrls; - org.joda.time.DateTime lastCertificateUpdateTime; - org.joda.time.DateTime lastExpiringCertNotificationSentDate; - org.joda.time.DateTime lastExpiringFailoverCertNotificationSentDate; -} -enum google.registry.model.registrar.Registrar$State { - ACTIVE; - DISABLED; - PENDING; - SUSPENDED; -} -enum google.registry.model.registrar.Registrar$Type { - EXTERNAL_MONITORING; - INTERNAL; - MONITORING; - OTE; - PDT; - REAL; - TEST; -} class google.registry.model.reporting.DomainTransactionRecord { google.registry.model.reporting.DomainTransactionRecord$TransactionReportField reportField; java.lang.Integer reportAmount;