Skip to content

Commit e4c4149

Browse files
authored
Remove more unused contact references (#2961)
This avoids changing any functionality, including the bits of DomainCommand (representations of XML files) that reference contacts. Currently, we "allow" parsing of contacts in DomainCommands and fail later as part of the domain flow, even though in practice the parsing itself will fail now that no contacts exist in the database. Because we wish to keep the "contacts aren't allowed in flows" tests active (e.g. DomainUpdateFlowTest::testFailure_minimumDataset_whenAddingNewContacts) we have to keep the usages of contacts in DomainCommand active for now.
1 parent e24c90f commit e4c4149

File tree

75 files changed

+501
-2370
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

75 files changed

+501
-2370
lines changed

core/src/main/java/google/registry/batch/DeleteLoadTestDataAction.java

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import google.registry.flows.poll.PollFlowUtils;
3030
import google.registry.model.EppResource;
3131
import google.registry.model.EppResourceUtils;
32-
import google.registry.model.contact.Contact;
3332
import google.registry.model.domain.Domain;
3433
import google.registry.model.host.Host;
3534
import google.registry.model.poll.PollMessage;
@@ -94,7 +93,6 @@ public void run() {
9493
TRANSACTION_REPEATABLE_READ,
9594
() -> {
9695
LOAD_TEST_REGISTRARS.forEach(this::deletePollMessages);
97-
tm().loadAllOfStream(Contact.class).forEach(this::deleteContact);
9896
tm().loadAllOfStream(Host.class).forEach(this::deleteHost);
9997
});
10098
}
@@ -110,21 +108,6 @@ private void deletePollMessages(String registrarId) {
110108
}
111109
}
112110

113-
private void deleteContact(Contact contact) {
114-
if (!LOAD_TEST_REGISTRARS.contains(contact.getPersistedCurrentSponsorRegistrarId())) {
115-
return;
116-
}
117-
// We cannot remove contacts from domains in the general case, so we cannot delete contacts
118-
// that are linked to domains (since it would break the foreign keys)
119-
if (EppResourceUtils.isLinked(contact.createVKey(), clock.nowUtc())) {
120-
logger.atWarning().log(
121-
"Cannot delete contact with repo ID %s since it is referenced from a domain.",
122-
contact.getRepoId());
123-
return;
124-
}
125-
deleteResource(contact);
126-
}
127-
128111
private void deleteHost(Host host) {
129112
if (!LOAD_TEST_REGISTRARS.contains(host.getPersistedCurrentSponsorRegistrarId())) {
130113
return;

core/src/main/java/google/registry/beam/resave/ResaveAllEppResourcesPipeline.java

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import google.registry.beam.common.RegistryJpaIO;
2626
import google.registry.beam.common.RegistryJpaIO.Read;
2727
import google.registry.model.EppResource;
28-
import google.registry.model.contact.Contact;
2928
import google.registry.model.domain.Domain;
3029
import google.registry.model.domain.DomainBase;
3130
import google.registry.model.host.Host;
@@ -56,7 +55,7 @@
5655
public class ResaveAllEppResourcesPipeline implements Serializable {
5756

5857
private static final ImmutableSet<Class<? extends EppResource>> EPP_RESOURCE_CLASSES =
59-
ImmutableSet.of(Contact.class, Domain.class, Host.class);
58+
ImmutableSet.of(Domain.class, Host.class);
6059

6160
/**
6261
* There exist three possible situations where we know we'll want to project domains to the
@@ -92,25 +91,12 @@ PipelineResult run() {
9291

9392
void setupPipeline(Pipeline pipeline) {
9493
if (options.getFast()) {
95-
fastResaveContacts(pipeline);
9694
fastResaveDomains(pipeline);
9795
} else {
9896
EPP_RESOURCE_CLASSES.forEach(clazz -> forceResaveAllResources(pipeline, clazz));
9997
}
10098
}
10199

102-
/** Projects to the current time and saves any contacts with expired transfers. */
103-
private void fastResaveContacts(Pipeline pipeline) {
104-
Read<String, String> repoIdRead =
105-
RegistryJpaIO.read(
106-
"SELECT repoId FROM Contact WHERE transferData.transferStatus = 'PENDING' AND"
107-
+ " transferData.pendingTransferExpirationTime < current_timestamp()",
108-
String.class,
109-
r -> r)
110-
.withCoder(StringUtf8Coder.of());
111-
projectAndResaveResources(pipeline, Contact.class, repoIdRead);
112-
}
113-
114100
/**
115101
* Projects to the current time and saves any domains with expired pending actions (e.g.
116102
* transfers, grace periods).

core/src/main/java/google/registry/flows/ResourceFlowUtils.java

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,14 @@
3434
import google.registry.flows.exceptions.TooManyResourceChecksException;
3535
import google.registry.model.EppResource;
3636
import google.registry.model.EppResource.ForeignKeyedEppResource;
37-
import google.registry.model.EppResource.ResourceWithTransferData;
3837
import google.registry.model.ForeignKeyUtils;
3938
import google.registry.model.domain.Domain;
4039
import google.registry.model.domain.DomainBase;
4140
import google.registry.model.domain.Period;
4241
import google.registry.model.domain.rgp.GracePeriodStatus;
4342
import google.registry.model.eppcommon.AuthInfo;
4443
import google.registry.model.eppcommon.StatusValue;
44+
import google.registry.model.host.Host;
4545
import google.registry.model.transfer.TransferStatus;
4646
import google.registry.persistence.VKey;
4747
import java.util.List;
@@ -63,30 +63,26 @@ public static void verifyResourceOwnership(String myRegistrarId, EppResource res
6363
}
6464
}
6565

66-
/**
67-
* Check whether if there are domains linked to the resource to be deleted. Throws an exception if
68-
* so.
69-
*/
70-
public static <R extends EppResource> void checkLinkedDomains(
71-
final String targetId, final DateTime now, final Class<R> resourceClass) throws EppException {
72-
VKey<R> key =
73-
ForeignKeyUtils.loadKey(resourceClass, targetId, now)
74-
.orElseThrow(() -> new ResourceDoesNotExistException(resourceClass, targetId));
66+
/** Check if there are domains linked to the host to be deleted. Throws an exception if so. */
67+
public static void checkLinkedDomains(final String targetId, final DateTime now)
68+
throws EppException {
69+
VKey<Host> key =
70+
ForeignKeyUtils.loadKey(Host.class, targetId, now)
71+
.orElseThrow(() -> new ResourceDoesNotExistException(Host.class, targetId));
7572
if (isLinked(key, now)) {
7673
throw new ResourceToDeleteIsReferencedException();
7774
}
7875
}
7976

80-
public static <R extends EppResource & ResourceWithTransferData> void verifyHasPendingTransfer(
81-
R resource) throws NotPendingTransferException {
82-
if (resource.getTransferData().getTransferStatus() != TransferStatus.PENDING) {
83-
throw new NotPendingTransferException(resource.getForeignKey());
77+
public static void verifyHasPendingTransfer(Domain domain) throws NotPendingTransferException {
78+
if (domain.getTransferData().getTransferStatus() != TransferStatus.PENDING) {
79+
throw new NotPendingTransferException(domain.getForeignKey());
8480
}
8581
}
8682

87-
public static <R extends EppResource & ResourceWithTransferData> void verifyTransferInitiator(
88-
String registrarId, R resource) throws NotTransferInitiatorException {
89-
if (!resource.getTransferData().getGainingRegistrarId().equals(registrarId)) {
83+
public static void verifyTransferInitiator(String registrarId, Domain domain)
84+
throws NotTransferInitiatorException {
85+
if (!domain.getTransferData().getGainingRegistrarId().equals(registrarId)) {
9086
throw new NotTransferInitiatorException();
9187
}
9288
}

core/src/main/java/google/registry/flows/host/HostDeleteFlow.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public EppResponse run() throws EppException {
8484
extensionManager.validate();
8585
DateTime now = tm().getTransactionTime();
8686
validateHostName(targetId);
87-
checkLinkedDomains(targetId, now, Host.class);
87+
checkLinkedDomains(targetId, now);
8888
Host existingHost = loadAndVerifyExistence(Host.class, targetId, now);
8989
verifyNoDisallowedStatuses(existingHost, ImmutableSet.of(StatusValue.PENDING_DELETE));
9090
if (!isSuperuser) {

core/src/main/java/google/registry/model/EppResourceUtils.java

Lines changed: 24 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,14 @@
2323

2424
import com.google.common.collect.ImmutableSet;
2525
import com.google.common.flogger.FluentLogger;
26-
import google.registry.model.EppResource.BuilderWithTransferData;
27-
import google.registry.model.EppResource.ResourceWithTransferData;
28-
import google.registry.model.contact.Contact;
2926
import google.registry.model.domain.Domain;
27+
import google.registry.model.domain.DomainBase;
3028
import google.registry.model.eppcommon.StatusValue;
3129
import google.registry.model.host.Host;
3230
import google.registry.model.reporting.HistoryEntry;
3331
import google.registry.model.reporting.HistoryEntryDao;
3432
import google.registry.model.tld.Tld;
3533
import google.registry.model.transfer.DomainTransferData;
36-
import google.registry.model.transfer.TransferData;
3734
import google.registry.model.transfer.TransferStatus;
3835
import google.registry.persistence.VKey;
3936
import jakarta.persistence.Query;
@@ -48,14 +45,6 @@ public final class EppResourceUtils {
4845

4946
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
5047

51-
private static final String CONTACT_LINKED_DOMAIN_QUERY =
52-
"SELECT repoId FROM Domain "
53-
+ "WHERE (adminContact = :fkRepoId "
54-
+ "OR billingContact = :fkRepoId "
55-
+ "OR techContact = :fkRepoId "
56-
+ "OR registrantContact = :fkRepoId) "
57-
+ "AND deletionTime > :now";
58-
5948
// We have to use the native SQL query here because DomainHost table doesn't have its entity
6049
// class, so we cannot reference its property like domainHost.hostRepoId in a JPQL query.
6150
private static final String HOST_LINKED_DOMAIN_QUERY =
@@ -105,41 +94,34 @@ public static boolean isDeleted(EppResource resource, DateTime time) {
10594
return !isActive(resource, time);
10695
}
10796

108-
/** Process an automatic transfer on a resource. */
109-
public static <
110-
T extends TransferData,
111-
B extends EppResource.Builder<?, B> & BuilderWithTransferData<T, B>>
112-
void setAutomaticTransferSuccessProperties(B builder, TransferData transferData) {
97+
/** Process an automatic transfer on a domain. */
98+
public static void setAutomaticTransferSuccessProperties(
99+
DomainBase.Builder<?, ?> builder, DomainTransferData transferData) {
113100
checkArgument(TransferStatus.PENDING.equals(transferData.getTransferStatus()));
114-
TransferData.Builder transferDataBuilder = transferData.asBuilder();
101+
DomainTransferData.Builder transferDataBuilder = transferData.asBuilder();
115102
transferDataBuilder.setTransferStatus(TransferStatus.SERVER_APPROVED);
116-
transferDataBuilder.setServerApproveEntities(null, null, null);
117-
if (transferData instanceof DomainTransferData) {
118-
((DomainTransferData.Builder) transferDataBuilder)
119-
.setServerApproveBillingEvent(null)
120-
.setServerApproveAutorenewEvent(null)
121-
.setServerApproveAutorenewPollMessage(null);
122-
}
103+
transferDataBuilder
104+
.setServerApproveEntities(null, null, null)
105+
.setServerApproveBillingEvent(null)
106+
.setServerApproveAutorenewEvent(null)
107+
.setServerApproveAutorenewPollMessage(null);
123108
builder
124109
.removeStatusValue(StatusValue.PENDING_TRANSFER)
125-
.setTransferData((T) transferDataBuilder.build())
110+
.setTransferData(transferDataBuilder.build())
126111
.setLastTransferTime(transferData.getPendingTransferExpirationTime())
127112
.setPersistedCurrentSponsorRegistrarId(transferData.getGainingRegistrarId());
128113
}
129114

130115
/**
131-
* Perform common operations for projecting an {@link EppResource} at a given time:
116+
* Perform common operations for projecting a {@link Domain} at a given time:
132117
*
133118
* <ul>
134119
* <li>Process an automatic transfer.
135120
* </ul>
136121
*/
137-
public static <
138-
T extends TransferData,
139-
E extends EppResource & ResourceWithTransferData<T>,
140-
B extends EppResource.Builder<?, B> & BuilderWithTransferData<T, B>>
141-
void projectResourceOntoBuilderAtTime(E resource, B builder, DateTime now) {
142-
T transferData = resource.getTransferData();
122+
public static void projectResourceOntoBuilderAtTime(
123+
DomainBase domain, DomainBase.Builder<?, ?> builder, DateTime now) {
124+
DomainTransferData transferData = domain.getTransferData();
143125
// If there's a pending transfer that has expired, process it.
144126
DateTime expirationTime = transferData.getPendingTransferExpirationTime();
145127
if (TransferStatus.PENDING.equals(transferData.getTransferStatus())
@@ -207,36 +189,21 @@ private static <T extends EppResource> T loadMostRecentRevisionAtTime(
207189
}
208190

209191
/**
210-
* Returns a set of {@link VKey} for domains that reference a specified contact or host.
211-
*
212-
* <p>This is an eventually consistent query if used for the database.
192+
* Returns a set of {@link VKey} for domains that reference a specified host.
213193
*
214194
* @param key the referent key
215195
* @param now the logical time of the check
216196
* @param limit the maximum number of returned keys, unlimited if null
217197
*/
218198
public static ImmutableSet<VKey<Domain>> getLinkedDomainKeys(
219-
VKey<? extends EppResource> key, DateTime now, @Nullable Integer limit) {
220-
checkArgument(
221-
key.getKind().equals(Contact.class) || key.getKind().equals(Host.class),
222-
"key must be either VKey<Contact> or VKey<Host>, but it is %s",
223-
key);
224-
boolean isContactKey = key.getKind().equals(Contact.class);
199+
VKey<Host> key, DateTime now, @Nullable Integer limit) {
225200
return tm().reTransact(
226201
() -> {
227-
Query query;
228-
if (isContactKey) {
229-
query =
230-
tm().query(CONTACT_LINKED_DOMAIN_QUERY, String.class)
231-
.setParameter("fkRepoId", key)
232-
.setParameter("now", now);
233-
} else {
234-
query =
235-
tm().getEntityManager()
236-
.createNativeQuery(HOST_LINKED_DOMAIN_QUERY)
237-
.setParameter("fkRepoId", key.getKey())
238-
.setParameter("now", now.toDate());
239-
}
202+
Query query =
203+
tm().getEntityManager()
204+
.createNativeQuery(HOST_LINKED_DOMAIN_QUERY)
205+
.setParameter("fkRepoId", key.getKey())
206+
.setParameter("now", now.toDate());
240207
if (limit != null) {
241208
query.setMaxResults(limit);
242209
}
@@ -252,12 +219,12 @@ public static ImmutableSet<VKey<Domain>> getLinkedDomainKeys(
252219
}
253220

254221
/**
255-
* Returns whether the given contact or host is linked to (that is, referenced by) a domain.
222+
* Returns whether the given host is linked to (that is, referenced by) a domain.
256223
*
257224
* @param key the referent key
258225
* @param now the logical time of the check
259226
*/
260-
public static boolean isLinked(VKey<? extends EppResource> key, DateTime now) {
227+
public static boolean isLinked(VKey<Host> key, DateTime now) {
261228
return !getLinkedDomainKeys(key, now, 1).isEmpty();
262229
}
263230

0 commit comments

Comments
 (0)