From 44972b916a42772e89147b3b19652a0e8f5175b7 Mon Sep 17 00:00:00 2001 From: Ben Kelsey Date: Fri, 6 Jan 2017 08:17:24 -0800 Subject: [PATCH] Add otherClientId to HistoryEntry This CL adds an otherClientId field to be populated on domain transfers with client ID of the other end of the transaction (losing registrar for requests and cancels, gaining registrar for approves and rejects). This will be used for reporting in compliance with specification 3 of the ICANN registry agreement. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=143775945 --- .../domain/DomainTransferApproveFlow.java | 5 +++-- .../flows/domain/DomainTransferCancelFlow.java | 1 + .../flows/domain/DomainTransferRejectFlow.java | 1 + .../domain/DomainTransferRequestFlow.java | 1 + .../registry/model/reporting/HistoryEntry.java | 18 ++++++++++++++++++ .../domain/DomainTransferApproveFlowTest.java | 3 +++ .../domain/DomainTransferCancelFlowTest.java | 5 +++++ .../domain/DomainTransferRejectFlowTest.java | 7 +++++++ .../domain/DomainTransferRequestFlowTest.java | 6 +++++- .../model/reporting/HistoryEntryTest.java | 1 + .../registry/testing/HistoryEntrySubject.java | 4 ++++ 11 files changed, 49 insertions(+), 3 deletions(-) diff --git a/java/google/registry/flows/domain/DomainTransferApproveFlow.java b/java/google/registry/flows/domain/DomainTransferApproveFlow.java index 3b9d3b297..86e311f1b 100644 --- a/java/google/registry/flows/domain/DomainTransferApproveFlow.java +++ b/java/google/registry/flows/domain/DomainTransferApproveFlow.java @@ -102,13 +102,14 @@ public final class DomainTransferApproveFlow implements TransactionalFlow { verifyResourceOwnership(clientId, existingDomain); String tld = existingDomain.getTld(); checkAllowedAccessToTld(clientId, tld); + TransferData transferData = existingDomain.getTransferData(); + String gainingClientId = transferData.getGainingClientId(); HistoryEntry historyEntry = historyBuilder .setType(HistoryEntry.Type.DOMAIN_TRANSFER_APPROVE) .setModificationTime(now) + .setOtherClientId(gainingClientId) .setParent(Key.create(existingDomain)) .build(); - TransferData transferData = existingDomain.getTransferData(); - String gainingClientId = transferData.getGainingClientId(); int extraYears = transferData.getExtendedRegistrationYears(); // Bill for the transfer. BillingEvent.OneTime billingEvent = new BillingEvent.OneTime.Builder() diff --git a/java/google/registry/flows/domain/DomainTransferCancelFlow.java b/java/google/registry/flows/domain/DomainTransferCancelFlow.java index 8c0edd6fe..f97a8f8ca 100644 --- a/java/google/registry/flows/domain/DomainTransferCancelFlow.java +++ b/java/google/registry/flows/domain/DomainTransferCancelFlow.java @@ -87,6 +87,7 @@ public final class DomainTransferCancelFlow implements TransactionalFlow { checkAllowedAccessToTld(clientId, existingDomain.getTld()); HistoryEntry historyEntry = historyBuilder .setType(HistoryEntry.Type.DOMAIN_TRANSFER_CANCEL) + .setOtherClientId(existingDomain.getTransferData().getLosingClientId()) .setModificationTime(now) .setParent(Key.create(existingDomain)) .build(); diff --git a/java/google/registry/flows/domain/DomainTransferRejectFlow.java b/java/google/registry/flows/domain/DomainTransferRejectFlow.java index c716d7b7c..1408e27f0 100644 --- a/java/google/registry/flows/domain/DomainTransferRejectFlow.java +++ b/java/google/registry/flows/domain/DomainTransferRejectFlow.java @@ -82,6 +82,7 @@ public final class DomainTransferRejectFlow implements TransactionalFlow { HistoryEntry historyEntry = historyBuilder .setType(HistoryEntry.Type.DOMAIN_TRANSFER_REJECT) .setModificationTime(now) + .setOtherClientId(existingDomain.getTransferData().getGainingClientId()) .setParent(Key.create(existingDomain)) .build(); verifyOptionalAuthInfo(authInfo, existingDomain); diff --git a/java/google/registry/flows/domain/DomainTransferRequestFlow.java b/java/google/registry/flows/domain/DomainTransferRequestFlow.java index 565c8eadd..ee2de70da 100644 --- a/java/google/registry/flows/domain/DomainTransferRequestFlow.java +++ b/java/google/registry/flows/domain/DomainTransferRequestFlow.java @@ -211,6 +211,7 @@ public final class DomainTransferRequestFlow implements TransactionalFlow { private HistoryEntry buildHistory(Period period, DomainResource existingResource, DateTime now) { return historyBuilder .setType(HistoryEntry.Type.DOMAIN_TRANSFER_REQUEST) + .setOtherClientId(existingResource.getCurrentSponsorClientId()) .setPeriod(period) .setModificationTime(now) .setParent(Key.create(existingResource)) diff --git a/java/google/registry/model/reporting/HistoryEntry.java b/java/google/registry/model/reporting/HistoryEntry.java index 6f0b10f5a..7488cdead 100644 --- a/java/google/registry/model/reporting/HistoryEntry.java +++ b/java/google/registry/model/reporting/HistoryEntry.java @@ -103,6 +103,15 @@ public class HistoryEntry extends ImmutableObject implements Buildable { @Index String clientId; + /** + * For transfers, the id of the other registrar. + * + *

For requests and cancels, the other registrar is the losing party (because the registrar + * sending the EPP transfer command is the gaining party). For approves and rejects, the other + * registrar is the gaining party. + */ + String otherClientId; + /** Transaction id that made this change. */ Trid trid; @@ -139,6 +148,10 @@ public class HistoryEntry extends ImmutableObject implements Buildable { return clientId; } + public String getOtherClientId() { + return otherClientId; + } + public Trid getTrid() { return trid; } @@ -203,6 +216,11 @@ public class HistoryEntry extends ImmutableObject implements Buildable { return this; } + public Builder setOtherClientId(String otherClientId) { + getInstance().otherClientId = otherClientId; + return this; + } + public Builder setTrid(Trid trid) { getInstance().trid = trid; return this; diff --git a/javatests/google/registry/flows/domain/DomainTransferApproveFlowTest.java b/javatests/google/registry/flows/domain/DomainTransferApproveFlowTest.java index ce886ef18..b3f3de567 100644 --- a/javatests/google/registry/flows/domain/DomainTransferApproveFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainTransferApproveFlowTest.java @@ -26,6 +26,7 @@ import static google.registry.testing.DatastoreHelper.getOnlyPollMessage; import static google.registry.testing.DatastoreHelper.getPollMessages; import static google.registry.testing.DatastoreHelper.persistResource; import static google.registry.testing.DomainResourceSubject.assertAboutDomains; +import static google.registry.testing.HistoryEntrySubject.assertAboutHistoryEntries; import static google.registry.testing.HostResourceSubject.assertAboutHosts; import static google.registry.util.DateTimeUtils.START_OF_TIME; import static java.util.Arrays.asList; @@ -155,6 +156,8 @@ public class DomainTransferApproveFlowTest HistoryEntry.Type.DOMAIN_TRANSFER_APPROVE); final HistoryEntry historyEntryTransferApproved = getOnlyHistoryEntryOfType(domain, HistoryEntry.Type.DOMAIN_TRANSFER_APPROVE); + assertAboutHistoryEntries().that(historyEntryTransferApproved) + .hasOtherClientId("NewRegistrar"); assertTransferApproved(domain); assertAboutDomains().that(domain).hasRegistrationExpirationTime(expectedExpirationTime); assertThat(ofy().load().key(domain.getAutorenewBillingEvent()).now().getEventTime()) diff --git a/javatests/google/registry/flows/domain/DomainTransferCancelFlowTest.java b/javatests/google/registry/flows/domain/DomainTransferCancelFlowTest.java index 6443ba449..50eaa5e69 100644 --- a/javatests/google/registry/flows/domain/DomainTransferCancelFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainTransferCancelFlowTest.java @@ -22,6 +22,7 @@ import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType; import static google.registry.testing.DatastoreHelper.getPollMessages; import static google.registry.testing.DatastoreHelper.persistResource; import static google.registry.testing.DomainResourceSubject.assertAboutDomains; +import static google.registry.testing.HistoryEntrySubject.assertAboutHistoryEntries; import static google.registry.util.DateTimeUtils.END_OF_TIME; import com.google.common.collect.ImmutableList; @@ -117,6 +118,10 @@ public class DomainTransferCancelFlowTest HistoryEntry.Type.DOMAIN_CREATE, HistoryEntry.Type.DOMAIN_TRANSFER_REQUEST, HistoryEntry.Type.DOMAIN_TRANSFER_CANCEL); + final HistoryEntry historyEntryTransferCancel = + getOnlyHistoryEntryOfType(domain, HistoryEntry.Type.DOMAIN_TRANSFER_CANCEL); + assertAboutHistoryEntries().that(historyEntryTransferCancel).hasClientId("NewRegistrar") + .and().hasOtherClientId("TheRegistrar"); // The only billing event left should be the original autorenew event, now reopened. assertBillingEvents( getLosingClientAutorenewEvent().asBuilder().setRecurrenceEndTime(END_OF_TIME).build()); diff --git a/javatests/google/registry/flows/domain/DomainTransferRejectFlowTest.java b/javatests/google/registry/flows/domain/DomainTransferRejectFlowTest.java index 2f96f9bfc..6c4f3dbb7 100644 --- a/javatests/google/registry/flows/domain/DomainTransferRejectFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainTransferRejectFlowTest.java @@ -17,10 +17,12 @@ package google.registry.flows.domain; import static com.google.common.truth.Truth.assertThat; import static google.registry.testing.DatastoreHelper.assertBillingEvents; import static google.registry.testing.DatastoreHelper.deleteResource; +import static google.registry.testing.DatastoreHelper.getOnlyHistoryEntryOfType; import static google.registry.testing.DatastoreHelper.getOnlyPollMessage; import static google.registry.testing.DatastoreHelper.getPollMessages; import static google.registry.testing.DatastoreHelper.persistResource; import static google.registry.testing.DomainResourceSubject.assertAboutDomains; +import static google.registry.testing.HistoryEntrySubject.assertAboutHistoryEntries; import static google.registry.util.DateTimeUtils.END_OF_TIME; import com.google.common.collect.FluentIterable; @@ -90,6 +92,11 @@ public class DomainTransferRejectFlowTest HistoryEntry.Type.DOMAIN_CREATE, HistoryEntry.Type.DOMAIN_TRANSFER_REQUEST, HistoryEntry.Type.DOMAIN_TRANSFER_REJECT); + final HistoryEntry historyEntryTransferRejected = + getOnlyHistoryEntryOfType(domain, HistoryEntry.Type.DOMAIN_TRANSFER_REJECT); + assertAboutHistoryEntries() + .that(historyEntryTransferRejected) + .hasOtherClientId("NewRegistrar"); // The only billing event left should be the original autorenew event, now reopened. assertBillingEvents( getLosingClientAutorenewEvent().asBuilder().setRecurrenceEndTime(END_OF_TIME).build()); diff --git a/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java b/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java index accb712ca..c8a0ed959 100644 --- a/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java @@ -190,7 +190,11 @@ public class DomainTransferRequestFlowTest .hasOneHistoryEntryEachOfTypes( HistoryEntry.Type.DOMAIN_CREATE, HistoryEntry.Type.DOMAIN_TRANSFER_REQUEST); - assertAboutHistoryEntries().that(historyEntryTransferRequest).hasPeriodYears(registrationYears); + assertAboutHistoryEntries() + .that(historyEntryTransferRequest) + .hasPeriodYears(registrationYears) + .and() + .hasOtherClientId("TheRegistrar"); assertAboutHosts().that(subordinateHost).hasNoHistoryEntries(); assertThat(getPollMessages("TheRegistrar", clock.nowUtc())).hasSize(1); diff --git a/javatests/google/registry/model/reporting/HistoryEntryTest.java b/javatests/google/registry/model/reporting/HistoryEntryTest.java index 1ae77f14b..0588969e6 100644 --- a/javatests/google/registry/model/reporting/HistoryEntryTest.java +++ b/javatests/google/registry/model/reporting/HistoryEntryTest.java @@ -43,6 +43,7 @@ public class HistoryEntryTest extends EntityTestCase { .setXmlBytes("".getBytes(UTF_8)) .setModificationTime(clock.nowUtc()) .setClientId("foo") + .setOtherClientId("otherClient") .setTrid(Trid.create("ABC-123")) .setBySuperuser(false) .setReason("reason") diff --git a/javatests/google/registry/testing/HistoryEntrySubject.java b/javatests/google/registry/testing/HistoryEntrySubject.java index 11126ebbc..7e18cb1ad 100644 --- a/javatests/google/registry/testing/HistoryEntrySubject.java +++ b/javatests/google/registry/testing/HistoryEntrySubject.java @@ -59,6 +59,10 @@ public class HistoryEntrySubject extends Subject hasOtherClientId(String otherClientId) { + return hasValue(otherClientId, actual().getOtherClientId(), "has other client ID"); + } + public And hasPeriod() { if (actual().getPeriod() == null) { fail("has a period");