mirror of
https://github.com/google/nomulus.git
synced 2025-05-15 17:07:15 +02:00
Filter cancellation records for only cancellable records
Previously, I would cancel all the records associated with HistoryEntry that's available for cancellation. This could cause unexpected behavior if we cancelled a historyEntry which itself had cancelled records (in effect we would negate the negation unintentionally). This is easily remedied by only cancelling records which want to be cancelled. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=167204383
This commit is contained in:
parent
d8c1501213
commit
3809ff59a5
5 changed files with 29 additions and 10 deletions
|
@ -960,10 +960,13 @@ public class DomainFlowUtils {
|
||||||
ImmutableSet.Builder<DomainTransactionRecord> recordsBuilder = new ImmutableSet.Builder<>();
|
ImmutableSet.Builder<DomainTransactionRecord> recordsBuilder = new ImmutableSet.Builder<>();
|
||||||
if (entryToCancel.isPresent()) {
|
if (entryToCancel.isPresent()) {
|
||||||
for (DomainTransactionRecord record : entryToCancel.get().getDomainTransactionRecords()) {
|
for (DomainTransactionRecord record : entryToCancel.get().getDomainTransactionRecords()) {
|
||||||
|
// Only cancel fields which are cancelable
|
||||||
|
if (cancelableFields.contains(record.getReportField())) {
|
||||||
int cancelledAmount = -1 * record.getReportAmount();
|
int cancelledAmount = -1 * record.getReportAmount();
|
||||||
recordsBuilder.add(record.asBuilder().setReportAmount(cancelledAmount).build());
|
recordsBuilder.add(record.asBuilder().setReportAmount(cancelledAmount).build());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
return recordsBuilder.build();
|
return recordsBuilder.build();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -870,17 +870,20 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow,
|
||||||
clock.advanceOneMilli();
|
clock.advanceOneMilli();
|
||||||
DomainTransactionRecord renewRecord =
|
DomainTransactionRecord renewRecord =
|
||||||
DomainTransactionRecord.create("tld", TIME_BEFORE_FLOW.plusDays(1), NET_RENEWS_3_YR, 1);
|
DomainTransactionRecord.create("tld", TIME_BEFORE_FLOW.plusDays(1), NET_RENEWS_3_YR, 1);
|
||||||
|
// We don't want to cancel non-add or renew records
|
||||||
|
DomainTransactionRecord notCancellableRecord =
|
||||||
|
DomainTransactionRecord.create("tld", TIME_BEFORE_FLOW.plusDays(1), RESTORED_DOMAINS, 5);
|
||||||
earlierHistoryEntry =
|
earlierHistoryEntry =
|
||||||
persistResource(
|
persistResource(
|
||||||
earlierHistoryEntry
|
earlierHistoryEntry
|
||||||
.asBuilder()
|
.asBuilder()
|
||||||
.setType(DOMAIN_CREATE)
|
.setType(DOMAIN_CREATE)
|
||||||
.setModificationTime(TIME_BEFORE_FLOW.minusDays(2))
|
.setModificationTime(TIME_BEFORE_FLOW.minusDays(2))
|
||||||
.setDomainTransactionRecords(ImmutableSet.of(renewRecord))
|
.setDomainTransactionRecords(ImmutableSet.of(renewRecord, notCancellableRecord))
|
||||||
.build());
|
.build());
|
||||||
runFlow();
|
runFlow();
|
||||||
HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_DELETE);
|
HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_DELETE);
|
||||||
// Transaction record should be the non-grace period delete, and the renew cancellation record
|
// We should only see the non-grace period delete record and the renew cancellation record
|
||||||
assertThat(persistedEntry.getDomainTransactionRecords())
|
assertThat(persistedEntry.getDomainTransactionRecords())
|
||||||
.containsExactly(
|
.containsExactly(
|
||||||
DomainTransactionRecord.create(
|
DomainTransactionRecord.create(
|
||||||
|
|
|
@ -17,6 +17,7 @@ package google.registry.flows.domain;
|
||||||
import static com.google.common.collect.Iterables.getOnlyElement;
|
import static com.google.common.collect.Iterables.getOnlyElement;
|
||||||
import static com.google.common.truth.Truth.assertThat;
|
import static com.google.common.truth.Truth.assertThat;
|
||||||
import static google.registry.model.ofy.ObjectifyService.ofy;
|
import static google.registry.model.ofy.ObjectifyService.ofy;
|
||||||
|
import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.NET_ADDS_4_YR;
|
||||||
import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.TRANSFER_SUCCESSFUL;
|
import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.TRANSFER_SUCCESSFUL;
|
||||||
import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_CREATE;
|
import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_CREATE;
|
||||||
import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_TRANSFER_APPROVE;
|
import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_TRANSFER_APPROVE;
|
||||||
|
@ -480,17 +481,20 @@ public class DomainTransferApproveFlowTest
|
||||||
DomainTransactionRecord previousSuccessRecord =
|
DomainTransactionRecord previousSuccessRecord =
|
||||||
DomainTransactionRecord.create(
|
DomainTransactionRecord.create(
|
||||||
"tld", clock.nowUtc().plusDays(1), TRANSFER_SUCCESSFUL, 1);
|
"tld", clock.nowUtc().plusDays(1), TRANSFER_SUCCESSFUL, 1);
|
||||||
|
// We only want to cancel TRANSFER_SUCCESSFUL records
|
||||||
|
DomainTransactionRecord notCancellableRecord =
|
||||||
|
DomainTransactionRecord.create("tld", clock.nowUtc().plusDays(1), NET_ADDS_4_YR, 5);
|
||||||
persistResource(
|
persistResource(
|
||||||
new HistoryEntry.Builder()
|
new HistoryEntry.Builder()
|
||||||
.setType(DOMAIN_TRANSFER_REQUEST)
|
.setType(DOMAIN_TRANSFER_REQUEST)
|
||||||
.setParent(domain)
|
.setParent(domain)
|
||||||
.setModificationTime(clock.nowUtc().minusDays(4))
|
.setModificationTime(clock.nowUtc().minusDays(4))
|
||||||
.setDomainTransactionRecords(
|
.setDomainTransactionRecords(
|
||||||
ImmutableSet.of(previousSuccessRecord))
|
ImmutableSet.of(previousSuccessRecord, notCancellableRecord))
|
||||||
.build());
|
.build());
|
||||||
runFlow();
|
runFlow();
|
||||||
HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_APPROVE);
|
HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_APPROVE);
|
||||||
// We should produce cancellation records for the original reporting date (now + 1 day) and
|
// We should only produce cancellation records for the original reporting date (now + 1 day) and
|
||||||
// success records for the new reporting date (now + transferGracePeriod=3 days)
|
// success records for the new reporting date (now + transferGracePeriod=3 days)
|
||||||
assertThat(persistedEntry.getDomainTransactionRecords())
|
assertThat(persistedEntry.getDomainTransactionRecords())
|
||||||
.containsExactly(
|
.containsExactly(
|
||||||
|
|
|
@ -15,6 +15,7 @@
|
||||||
package google.registry.flows.domain;
|
package google.registry.flows.domain;
|
||||||
|
|
||||||
import static com.google.common.truth.Truth.assertThat;
|
import static com.google.common.truth.Truth.assertThat;
|
||||||
|
import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.RESTORED_DOMAINS;
|
||||||
import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.TRANSFER_SUCCESSFUL;
|
import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.TRANSFER_SUCCESSFUL;
|
||||||
import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_CREATE;
|
import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_CREATE;
|
||||||
import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_TRANSFER_CANCEL;
|
import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_TRANSFER_CANCEL;
|
||||||
|
@ -351,16 +352,20 @@ public class DomainTransferCancelFlowTest
|
||||||
.build());
|
.build());
|
||||||
DomainTransactionRecord previousSuccessRecord =
|
DomainTransactionRecord previousSuccessRecord =
|
||||||
DomainTransactionRecord.create("tld", clock.nowUtc().plusDays(1), TRANSFER_SUCCESSFUL, 1);
|
DomainTransactionRecord.create("tld", clock.nowUtc().plusDays(1), TRANSFER_SUCCESSFUL, 1);
|
||||||
|
// We only want to cancel TRANSFER_SUCCESSFUL records
|
||||||
|
DomainTransactionRecord notCancellableRecord =
|
||||||
|
DomainTransactionRecord.create("tld", clock.nowUtc().plusDays(1), RESTORED_DOMAINS, 5);
|
||||||
persistResource(
|
persistResource(
|
||||||
new HistoryEntry.Builder()
|
new HistoryEntry.Builder()
|
||||||
.setType(DOMAIN_TRANSFER_REQUEST)
|
.setType(DOMAIN_TRANSFER_REQUEST)
|
||||||
.setParent(domain)
|
.setParent(domain)
|
||||||
.setModificationTime(clock.nowUtc().minusDays(4))
|
.setModificationTime(clock.nowUtc().minusDays(4))
|
||||||
.setDomainTransactionRecords(ImmutableSet.of(previousSuccessRecord))
|
.setDomainTransactionRecords(
|
||||||
|
ImmutableSet.of(previousSuccessRecord, notCancellableRecord))
|
||||||
.build());
|
.build());
|
||||||
runFlow();
|
runFlow();
|
||||||
HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_CANCEL);
|
HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_CANCEL);
|
||||||
// We should produce a cancellation record for the original transfer success
|
// We should only produce a cancellation record for the original transfer success
|
||||||
assertThat(persistedEntry.getDomainTransactionRecords())
|
assertThat(persistedEntry.getDomainTransactionRecords())
|
||||||
.containsExactly(previousSuccessRecord.asBuilder().setReportAmount(-1).build());
|
.containsExactly(previousSuccessRecord.asBuilder().setReportAmount(-1).build());
|
||||||
}
|
}
|
||||||
|
|
|
@ -15,6 +15,7 @@
|
||||||
package google.registry.flows.domain;
|
package google.registry.flows.domain;
|
||||||
|
|
||||||
import static com.google.common.truth.Truth.assertThat;
|
import static com.google.common.truth.Truth.assertThat;
|
||||||
|
import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.NET_RENEWS_3_YR;
|
||||||
import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.TRANSFER_NACKED;
|
import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.TRANSFER_NACKED;
|
||||||
import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.TRANSFER_SUCCESSFUL;
|
import static google.registry.model.reporting.DomainTransactionRecord.TransactionReportField.TRANSFER_SUCCESSFUL;
|
||||||
import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_CREATE;
|
import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_CREATE;
|
||||||
|
@ -317,17 +318,20 @@ public class DomainTransferRejectFlowTest
|
||||||
DomainTransactionRecord previousSuccessRecord =
|
DomainTransactionRecord previousSuccessRecord =
|
||||||
DomainTransactionRecord.create(
|
DomainTransactionRecord.create(
|
||||||
"tld", clock.nowUtc().plusDays(1), TRANSFER_SUCCESSFUL, 1);
|
"tld", clock.nowUtc().plusDays(1), TRANSFER_SUCCESSFUL, 1);
|
||||||
|
// We only want to cancel TRANSFER_SUCCESSFUL records
|
||||||
|
DomainTransactionRecord notCancellableRecord =
|
||||||
|
DomainTransactionRecord.create("tld", clock.nowUtc().plusDays(1), NET_RENEWS_3_YR, 5);
|
||||||
persistResource(
|
persistResource(
|
||||||
new HistoryEntry.Builder()
|
new HistoryEntry.Builder()
|
||||||
.setType(DOMAIN_TRANSFER_REQUEST)
|
.setType(DOMAIN_TRANSFER_REQUEST)
|
||||||
.setParent(domain)
|
.setParent(domain)
|
||||||
.setModificationTime(clock.nowUtc().minusDays(4))
|
.setModificationTime(clock.nowUtc().minusDays(4))
|
||||||
.setDomainTransactionRecords(
|
.setDomainTransactionRecords(
|
||||||
ImmutableSet.of(previousSuccessRecord))
|
ImmutableSet.of(previousSuccessRecord, notCancellableRecord))
|
||||||
.build());
|
.build());
|
||||||
runFlow();
|
runFlow();
|
||||||
HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_REJECT);
|
HistoryEntry persistedEntry = getOnlyHistoryEntryOfType(domain, DOMAIN_TRANSFER_REJECT);
|
||||||
// We should produce cancellation records for the original success records and nack records
|
// We should only produce cancellation records for the original success records and nack records
|
||||||
assertThat(persistedEntry.getDomainTransactionRecords())
|
assertThat(persistedEntry.getDomainTransactionRecords())
|
||||||
.containsExactly(
|
.containsExactly(
|
||||||
previousSuccessRecord.asBuilder().setReportAmount(-1).build(),
|
previousSuccessRecord.asBuilder().setReportAmount(-1).build(),
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue