Permalink
Browse files

Add detailed log when DuplicateContactForRoleException is thrown

This change also added a test to verify that EPP request to modify
both contacts and registrant at same time can be handled as expected.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=232935690
  • Loading branch information...
hstonec authored and gbrodman committed Feb 7, 2019
1 parent 1c837dc commit 71d65ed73a361ffbeea6fbd184031808d23e1b7b
@@ -18,6 +18,7 @@
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.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.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.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 @@ static void validateNameserversCountForTld(String tld, InternetDomainName domain

static void validateNoDuplicateContacts(Set<DesignatedContact> contacts)
throws ParameterValuePolicyErrorException {
Set<Type> roles = new HashSet<>();
for (DesignatedContact contact : nullToEmpty(contacts)) {
if (!roles.add(contact.getType())) {
throw new DuplicateContactForRoleException();
}
Map<Type, Collection<String>> 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 MissingContactTypeException() {

/** 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<Type, Collection<String>> roleToDupContacts) {
super(constructErrorMessageFrom(roleToDupContacts));
}
}

private static String constructErrorMessageFrom(Map<Type, Collection<String>> 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<Type, Collection<String>> entry) {
return String.format(
"contacts [%s] have same role [%s]",
Joiner.on(", ").join(sort(entry.getValue())), Ascii.toLowerCase(entry.getKey().toString()));
}

private static ImmutableSortedSet<Comparable<?>> sort(Collection<String> 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() {
@@ -120,6 +120,31 @@ private void persistReferencedEntities() {
unusedContact = persistActiveContact("unused");
}

private DomainBase persistDomainWithRegistrant() throws Exception {
HostResource host =
loadByForeignKey(HostResource.class, "ns1.example.foo", clock.nowUtc()).get();
DomainBase domain =
persistResource(
newDomainBase(getUniqueIdFromCommand())
.asBuilder()
.setContacts(
ImmutableSet.of(
DesignatedContact.create(Type.TECH, Key.create(mak21Contact)),
DesignatedContact.create(Type.ADMIN, Key.create(mak21Contact)),
DesignatedContact.create(Type.BILLING, Key.create(mak21Contact))))
.setRegistrant(Key.create(mak21Contact))
.setNameservers(ImmutableSet.of(Key.create(host)))
.build());
historyEntryDomainCreate =
persistResource(
new HistoryEntry.Builder()
.setType(HistoryEntry.Type.DOMAIN_CREATE)
.setParent(domain)
.build());
clock.advanceOneMilli();
return domain;
}

private DomainBase persistDomain() throws Exception {
HostResource host =
loadByForeignKey(HostResource.class, "ns1.example.foo", clock.nowUtc()).get();
@@ -758,6 +783,10 @@ public void testFailure_addingDuplicateContact() throws Exception {
.build());
EppException thrown = assertThrows(DuplicateContactForRoleException.class, this::runFlow);
assertAboutEppExceptions().that(thrown).marshalsToXml();
assertThat(thrown.getResult().getMsg())
.isEqualTo(
"More than one contact for a given role is not allowed: "
+ "contacts [foo, mak21] have same role [tech]");
}

@Test
@@ -868,6 +897,19 @@ public void testFailure_duplicateContactInCommand() throws Exception {
assertAboutEppExceptions().that(thrown).marshalsToXml();
}

@Test
public void testFailure_multipleDuplicateContactInCommand() throws Exception {
setEppInput("domain_update_multiple_duplicate_contacts.xml");
persistReferencedEntities();
persistDomain();
EppException thrown = assertThrows(DuplicateContactForRoleException.class, this::runFlow);
assertThat(thrown.getMessage())
.isEqualTo("More than one contact for a given role is not allowed: "
+ "contacts [mak21, sh8013] have same role [billing], "
+ "contacts [mak21, sh8013] have same role [tech]");
assertAboutEppExceptions().that(thrown).marshalsToXml();
}

@Test
public void testFailure_missingContactType() throws Exception {
// We need to test for missing type, but not for invalid - the schema enforces that for us.
@@ -1091,6 +1133,35 @@ public void testSuccess_changeRegistrantWhitelisted() throws Exception {
.isEqualTo("sh8013");
}

@Test
public void testSuccess_changeContactsAndRegistrant() throws Exception {
setEppInput("domain_update_contacts_and_registrant.xml");
persistReferencedEntities();
persistDomainWithRegistrant();

reloadResourceByForeignKey()
.getContacts()
.forEach(
contact -> {
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();
@@ -0,0 +1,24 @@
<?xml version="1.0" encoding="UTF-8"?>
<epp xmlns="urn:ietf:params:xml:ns:epp-1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="urn:ietf:params:xml:ns:epp-1.0 epp-1.0.xsd">
<command>
<update>
<domain:update xmlns:domain="urn:ietf:params:xml:ns:domain-1.0" xsi:schemaLocation="urn:ietf:params:xml:ns:domain-1.0 domain-1.0.xsd">
<domain:name>example.tld</domain:name>
<domain:add>
<domain:contact type="admin">sh8013</domain:contact>
<domain:contact type="tech">sh8013</domain:contact>
<domain:contact type="billing">sh8013</domain:contact>
</domain:add>
<domain:rem>
<domain:contact type="admin">mak21</domain:contact>
<domain:contact type="tech">mak21</domain:contact>
<domain:contact type="billing">mak21</domain:contact>
</domain:rem>
<domain:chg>
<domain:registrant>sh8013</domain:registrant>
</domain:chg>
</domain:update>
</update>
<clTRID>ABC-12345</clTRID>
</command>
</epp>
@@ -0,0 +1,34 @@
<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
<command>
<update>
<domain:update
xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
<domain:name>example.tld</domain:name>
<domain:add>
<domain:ns>
<domain:hostObj>ns2.example.foo</domain:hostObj>
</domain:ns>
<domain:contact type="tech">mak21</domain:contact>
<domain:contact type="tech">sh8013</domain:contact>
<domain:contact type="billing">mak21</domain:contact>
<domain:contact type="billing">sh8013</domain:contact>
<domain:status s="clientHold"
lang="en">Payment overdue.</domain:status>
</domain:add>
<domain:rem>
<domain:ns>
<domain:hostObj>ns1.example.foo</domain:hostObj>
</domain:ns>
<domain:status s="clientUpdateProhibited"/>
</domain:rem>
<domain:chg>
<domain:registrant>sh8013</domain:registrant>
<domain:authInfo>
<domain:pw>2BARfoo</domain:pw>
</domain:authInfo>
</domain:chg>
</domain:update>
</update>
<clTRID>ABC-12345</clTRID>
</command>
</epp>

0 comments on commit 71d65ed

Please sign in to comment.