diff --git a/java/google/registry/flows/domain/DomainFlowUtils.java b/java/google/registry/flows/domain/DomainFlowUtils.java index 33c1a5c5b..c07f04b5b 100644 --- a/java/google/registry/flows/domain/DomainFlowUtils.java +++ b/java/google/registry/flows/domain/DomainFlowUtils.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Predicates.equalTo; import static com.google.common.collect.ImmutableMap.toImmutableMap; +import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap; import static com.google.common.collect.Iterables.any; import static com.google.common.collect.Sets.difference; import static com.google.common.collect.Sets.intersection; @@ -41,6 +42,7 @@ import static google.registry.util.DateTimeUtils.isAtOrAfter; import static google.registry.util.DateTimeUtils.leapSafeAddYears; import static google.registry.util.DomainNameUtils.ACE_PREFIX; +import com.google.common.base.Ascii; import com.google.common.base.CharMatcher; import com.google.common.base.Joiner; import com.google.common.base.Splitter; @@ -49,6 +51,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Sets; import com.google.common.collect.Streams; import com.google.common.net.InternetDomainName; @@ -116,9 +119,11 @@ import google.registry.model.tmch.ClaimsListShard; import google.registry.tldconfig.idn.IdnLabelValidator; import google.registry.util.Idn; import java.math.BigDecimal; +import java.util.Collection; import java.util.Comparator; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Map.Entry; import java.util.Optional; import java.util.Set; @@ -360,11 +365,20 @@ public class DomainFlowUtils { static void validateNoDuplicateContacts(Set contacts) throws ParameterValuePolicyErrorException { - Set roles = new HashSet<>(); - for (DesignatedContact contact : nullToEmpty(contacts)) { - if (!roles.add(contact.getType())) { - throw new DuplicateContactForRoleException(); - } + Map> roleToDupContacts = + nullToEmpty(contacts).stream() + .collect( + toImmutableSetMultimap( + contact -> contact.getType(), + contact -> ofy().load().key(contact.getContactKey()).now().getContactId())) + .asMap() + .entrySet() + .stream() + .filter(entry -> entry.getValue().size() >= 2) + .collect(Collectors.toMap(entry -> entry.getKey(), entry -> entry.getValue())); + + if (!roleToDupContacts.isEmpty()) { + throw new DuplicateContactForRoleException(roleToDupContacts); } } @@ -1140,11 +1154,32 @@ public class DomainFlowUtils { /** More than one contact for a given role is not allowed. */ static class DuplicateContactForRoleException extends ParameterValuePolicyErrorException { - public DuplicateContactForRoleException() { - super("More than one contact for a given role is not allowed"); + public DuplicateContactForRoleException(Map> roleToDupContacts) { + super(constructErrorMessageFrom(roleToDupContacts)); } } + private static String constructErrorMessageFrom(Map> roleToDupContacts) { + String detailError = + Joiner.on(", ") + .join( + sort( + roleToDupContacts.entrySet().stream() + .map(DomainFlowUtils::formatDetailMessage) + .collect(Collectors.toSet()))); + return String.format("More than one contact for a given role is not allowed: %s", detailError); + } + + private static String formatDetailMessage(Map.Entry> entry) { + return String.format( + "contacts [%s] have same role [%s]", + Joiner.on(", ").join(sort(entry.getValue())), Ascii.toLowerCase(entry.getKey().toString())); + } + + private static ImmutableSortedSet> sort(Collection unsorted) { + return ImmutableSortedSet.naturalOrder().addAll(unsorted).build(); + } + /** Declared launch extension phase does not match the current registry phase. */ static class LaunchPhaseMismatchException extends ParameterValuePolicyErrorException { public LaunchPhaseMismatchException() { diff --git a/javatests/google/registry/flows/domain/DomainUpdateFlowTest.java b/javatests/google/registry/flows/domain/DomainUpdateFlowTest.java index 5a27dceb8..511d94207 100644 --- a/javatests/google/registry/flows/domain/DomainUpdateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainUpdateFlowTest.java @@ -120,6 +120,31 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase { + assertThat(ofy().load().key(contact.getContactKey()).now().getContactId()) + .isEqualTo("mak21"); + }); + assertThat(ofy().load().key(reloadResourceByForeignKey().getRegistrant()).now().getContactId()) + .isEqualTo("mak21"); + + runFlow(); + + reloadResourceByForeignKey() + .getContacts() + .forEach( + contact -> { + assertThat(ofy().load().key(contact.getContactKey()).now().getContactId()) + .isEqualTo("sh8013"); + }); + assertThat(ofy().load().key(reloadResourceByForeignKey().getRegistrant()).now().getContactId()) + .isEqualTo("sh8013"); + } + @Test public void testSuccess_nameserverAndRegistrantWhitelisted() throws Exception { persistReferencedEntities(); diff --git a/javatests/google/registry/flows/domain/testdata/domain_update_contacts_and_registrant.xml b/javatests/google/registry/flows/domain/testdata/domain_update_contacts_and_registrant.xml new file mode 100644 index 000000000..b45b24a81 --- /dev/null +++ b/javatests/google/registry/flows/domain/testdata/domain_update_contacts_and_registrant.xml @@ -0,0 +1,24 @@ + + + + + + example.tld + + sh8013 + sh8013 + sh8013 + + + mak21 + mak21 + mak21 + + + sh8013 + + + + ABC-12345 + + diff --git a/javatests/google/registry/flows/domain/testdata/domain_update_multiple_duplicate_contacts.xml b/javatests/google/registry/flows/domain/testdata/domain_update_multiple_duplicate_contacts.xml new file mode 100644 index 000000000..e0e2c9e84 --- /dev/null +++ b/javatests/google/registry/flows/domain/testdata/domain_update_multiple_duplicate_contacts.xml @@ -0,0 +1,34 @@ + + + + + example.tld + + + ns2.example.foo + + mak21 + sh8013 + mak21 + sh8013 + Payment overdue. + + + + ns1.example.foo + + + + + sh8013 + + 2BARfoo + + + + + ABC-12345 + +