Use StatusValue XML names in EPP error messages

This changes ResourceStatusProhibitsOperationException so that we print out the list of StatusValues using their XML names rather than the literal enum name, i.e. we use "pendingDelete" rather than "PENDING_DELETE".

This seems more correct given that EPP clients will be used to seeing the status values in the XML representation, and it also matches the existing ResourceHasClientUpdateProhibitedException that hardcodes "clientUpdateProhibited":
http://[]/third_party/java_src/gtld/java/google/registry/flows/exceptions/ResourceHasClientUpdateProhibitedException.java?l=22&rcl=146111211

Also reorganized related test methods and added some missing tests, including for ContactTransferRequestFlow which previously had none.  I also renamed the "clientProhibitedStatusValue" tests to instead say "statusValueNotClientSettable" to be clearer about what's being tested, and that it's not related to the "clientXXProhibited" statuses.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=150248562
This commit is contained in:
nickfelt 2017-03-15 15:11:30 -07:00 committed by Ben McIlwain
parent e60c01c2f7
commit 6a8b25360c
13 changed files with 197 additions and 120 deletions

View file

@ -50,6 +50,8 @@ or rejected, and the gaining registrar can also cancel the transfer request.
* The resource is already pending transfer.
* 2303
* Resource with this id does not exist.
* 2304
* Resource status prohibits this operation.
## ContactTransferRejectFlow

View file

@ -64,6 +64,7 @@ import org.joda.time.Duration;
* @error {@link google.registry.flows.exceptions.AlreadyPendingTransferException}
* @error {@link google.registry.flows.exceptions.MissingTransferRequestAuthInfoException}
* @error {@link google.registry.flows.exceptions.ObjectAlreadySponsoredException}
* @error {@link google.registry.flows.exceptions.ResourceStatusProhibitsOperationException}
*/
public final class ContactTransferRequestFlow implements TransactionalFlow {

View file

@ -15,6 +15,7 @@
package google.registry.flows.exceptions;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableSet;
import google.registry.flows.EppException.StatusProhibitsOperationException;
import google.registry.model.eppcommon.StatusValue;
import java.util.Set;
@ -22,7 +23,16 @@ import java.util.Set;
/** Resource status prohibits this operation. */
public class ResourceStatusProhibitsOperationException
extends StatusProhibitsOperationException {
public ResourceStatusProhibitsOperationException(Set<StatusValue> status) {
super("Operation disallowed by status: " + Joiner.on(", ").join(status));
public ResourceStatusProhibitsOperationException(Set<StatusValue> statuses) {
super("Operation disallowed by status: " + formatStatusValues(statuses));
}
/** Returns a human-readable string listing the XML names of the given status values. */
private static String formatStatusValues(Set<StatusValue> statuses) {
ImmutableSet.Builder<String> statusXmlNames = new ImmutableSet.Builder<>();
for (StatusValue status : statuses) {
statusXmlNames.add(status.getXmlName());
}
return Joiner.on(", ").join(statusXmlNames.build());
}
}

View file

@ -44,16 +44,6 @@ public class ContactDeleteFlowTest
setEppInput("contact_delete.xml");
}
private void doFailingStatusTest(StatusValue statusValue, Class<? extends Exception> exception)
throws Exception {
persistResource(
newContactResource(getUniqueIdFromCommand()).asBuilder()
.setStatusValues(ImmutableSet.of(statusValue))
.build());
thrown.expect(exception);
runFlow();
}
@Test
public void testDryRun() throws Exception {
persistActiveContact(getUniqueIdFromCommand());
@ -92,6 +82,16 @@ public class ContactDeleteFlowTest
runFlow();
}
private void doFailingStatusTest(StatusValue statusValue, Class<? extends Exception> exception)
throws Exception {
persistResource(
newContactResource(getUniqueIdFromCommand()).asBuilder()
.setStatusValues(ImmutableSet.of(statusValue))
.build());
thrown.expect(exception, statusValue.getXmlName());
runFlow();
}
@Test
public void testFailure_existedButWasClientDeleteProhibited() throws Exception {
doFailingStatusTest(

View file

@ -28,9 +28,11 @@ import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException;
import google.registry.flows.exceptions.AlreadyPendingTransferException;
import google.registry.flows.exceptions.MissingTransferRequestAuthInfoException;
import google.registry.flows.exceptions.ObjectAlreadySponsoredException;
import google.registry.flows.exceptions.ResourceStatusProhibitsOperationException;
import google.registry.model.contact.ContactAuthInfo;
import google.registry.model.contact.ContactResource;
import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
import google.registry.model.eppcommon.StatusValue;
import google.registry.model.reporting.HistoryEntry;
import google.registry.model.transfer.TransferStatus;
import org.joda.time.DateTime;
@ -187,4 +189,28 @@ public class ContactTransferRequestFlowTest
String.format("(%s)", getUniqueIdFromCommand()));
doFailingTest("contact_transfer_request.xml");
}
@Test
public void testFailure_clientTransferProhibited() throws Exception {
contact = persistResource(
contact.asBuilder().addStatusValue(StatusValue.CLIENT_TRANSFER_PROHIBITED).build());
thrown.expect(ResourceStatusProhibitsOperationException.class, "clientTransferProhibited");
doFailingTest("contact_transfer_request.xml");
}
@Test
public void testFailure_serverTransferProhibited() throws Exception {
contact = persistResource(
contact.asBuilder().addStatusValue(StatusValue.SERVER_TRANSFER_PROHIBITED).build());
thrown.expect(ResourceStatusProhibitsOperationException.class, "serverTransferProhibited");
doFailingTest("contact_transfer_request.xml");
}
@Test
public void testFailure_pendingDelete() throws Exception {
contact = persistResource(
contact.asBuilder().addStatusValue(StatusValue.PENDING_DELETE).build());
thrown.expect(ResourceStatusProhibitsOperationException.class, "pendingDelete");
doFailingTest("contact_transfer_request.xml");
}
}

View file

@ -71,18 +71,6 @@ public class ContactUpdateFlowTest
doSuccessfulTest();
}
@Test
public void testSuccess_removeClientUpdateProhibited() throws Exception {
setEppInput("contact_update_remove_client_update_prohibited.xml");
persistResource(
newContactResource(getUniqueIdFromCommand()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.CLIENT_UPDATE_PROHIBITED))
.build());
doSuccessfulTest();
assertAboutContacts().that(reloadResourceByForeignKey())
.doesNotHaveStatusValue(StatusValue.CLIENT_UPDATE_PROHIBITED);
}
@Test
public void testSuccess_updatingInternationalizedPostalInfoDeletesLocalized() throws Exception {
ContactResource contact =
@ -281,7 +269,7 @@ public class ContactUpdateFlowTest
}
@Test
public void testFailure_clientProhibitedStatusValue() throws Exception {
public void testFailure_statusValueNotClientSettable() throws Exception {
setEppInput("contact_update_prohibited_status.xml");
persistActiveContact(getUniqueIdFromCommand());
thrown.expect(StatusNotClientSettableException.class);
@ -289,7 +277,7 @@ public class ContactUpdateFlowTest
}
@Test
public void testSuccess_superuserClientProhibitedStatusValue() throws Exception {
public void testSuccess_superuserStatusValueNotClientSettable() throws Exception {
setEppInput("contact_update_prohibited_status.xml");
persistActiveContact(getUniqueIdFromCommand());
clock.advanceOneMilli();
@ -317,7 +305,19 @@ public class ContactUpdateFlowTest
}
@Test
public void testSuccess_superuserClientUpdateProhibited() throws Exception {
public void testSuccess_clientUpdateProhibited_removed() throws Exception {
setEppInput("contact_update_remove_client_update_prohibited.xml");
persistResource(
newContactResource(getUniqueIdFromCommand()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.CLIENT_UPDATE_PROHIBITED))
.build());
doSuccessfulTest();
assertAboutContacts().that(reloadResourceByForeignKey())
.doesNotHaveStatusValue(StatusValue.CLIENT_UPDATE_PROHIBITED);
}
@Test
public void testSuccess_superuserClientUpdateProhibited_notRemoved() throws Exception {
setEppInput("contact_update_prohibited_status.xml");
persistResource(
newContactResource(getUniqueIdFromCommand()).asBuilder()
@ -334,7 +334,7 @@ public class ContactUpdateFlowTest
}
@Test
public void testFailure_clientUpdateProhibited() throws Exception {
public void testFailure_clientUpdateProhibited_notRemoved() throws Exception {
persistResource(
newContactResource(getUniqueIdFromCommand()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.CLIENT_UPDATE_PROHIBITED))
@ -349,7 +349,17 @@ public class ContactUpdateFlowTest
newContactResource(getUniqueIdFromCommand()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.SERVER_UPDATE_PROHIBITED))
.build());
thrown.expect(ResourceStatusProhibitsOperationException.class);
thrown.expect(ResourceStatusProhibitsOperationException.class, "serverUpdateProhibited");
runFlow();
}
@Test
public void testFailure_pendingDeleteProhibited() throws Exception {
persistResource(
newContactResource(getUniqueIdFromCommand()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.PENDING_DELETE))
.build());
thrown.expect(ResourceStatusProhibitsOperationException.class, "pendingDelete");
runFlow();
}

View file

@ -444,7 +444,7 @@ public class DomainApplicationUpdateFlowTest
persistReferencedEntities();
persistResource(newApplicationBuilder().setStatusValues(
ImmutableSet.of(StatusValue.SERVER_UPDATE_PROHIBITED)).build());
thrown.expect(ResourceStatusProhibitsOperationException.class);
thrown.expect(ResourceStatusProhibitsOperationException.class, "serverUpdateProhibited");
runFlow();
}

View file

@ -690,7 +690,7 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow,
persistResource(newDomainResource(getUniqueIdFromCommand()).asBuilder()
.addStatusValue(StatusValue.CLIENT_DELETE_PROHIBITED)
.build());
thrown.expect(ResourceStatusProhibitsOperationException.class);
thrown.expect(ResourceStatusProhibitsOperationException.class, "clientDeleteProhibited");
runFlow();
}
@ -699,7 +699,7 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow,
persistResource(newDomainResource(getUniqueIdFromCommand()).asBuilder()
.addStatusValue(StatusValue.SERVER_DELETE_PROHIBITED)
.build());
thrown.expect(ResourceStatusProhibitsOperationException.class);
thrown.expect(ResourceStatusProhibitsOperationException.class, "serverDeleteProhibited");
runFlow();
}
@ -708,7 +708,7 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow,
persistResource(newDomainResource(getUniqueIdFromCommand()).asBuilder()
.addStatusValue(StatusValue.PENDING_DELETE)
.build());
thrown.expect(ResourceStatusProhibitsOperationException.class);
thrown.expect(ResourceStatusProhibitsOperationException.class, "pendingDelete");
runFlow();
}

View file

@ -427,14 +427,25 @@ public class DomainRenewFlowTest extends ResourceFlowTestCase<DomainRenewFlow, D
@Test
public void testFailure_clientRenewProhibited() throws Exception {
persistDomain(StatusValue.CLIENT_RENEW_PROHIBITED);
thrown.expect(ResourceStatusProhibitsOperationException.class);
thrown.expect(ResourceStatusProhibitsOperationException.class, "clientRenewProhibited");
runFlow();
}
@Test
public void testFailure_serverRenewProhibited() throws Exception {
persistDomain(StatusValue.SERVER_RENEW_PROHIBITED);
thrown.expect(ResourceStatusProhibitsOperationException.class);
thrown.expect(ResourceStatusProhibitsOperationException.class, "serverRenewProhibited");
runFlow();
}
@Test
public void testFailure_pendingDelete() throws Exception {
persistResource(newDomainResource(getUniqueIdFromCommand()).asBuilder()
.setRegistrationExpirationTime(expirationTime)
.setDeletionTime(clock.nowUtc().plusDays(1))
.addStatusValue(StatusValue.PENDING_DELETE)
.build());
thrown.expect(ResourceStatusProhibitsOperationException.class, "pendingDelete");
runFlow();
}
@ -582,17 +593,6 @@ public class DomainRenewFlowTest extends ResourceFlowTestCase<DomainRenewFlow, D
runFlow();
}
@Test
public void testFailure_pendingDelete() throws Exception {
persistResource(newDomainResource(getUniqueIdFromCommand()).asBuilder()
.setRegistrationExpirationTime(expirationTime)
.setDeletionTime(clock.nowUtc().plusDays(1))
.addStatusValue(StatusValue.PENDING_DELETE)
.build());
thrown.expect(ResourceStatusProhibitsOperationException.class);
runFlow();
}
@Test
public void testFailure_curExpDateMustMatch() throws Exception {
persistDomain();

View file

@ -924,12 +924,30 @@ public class DomainTransferRequestFlowTest
doFailingTest("domain_transfer_request_months.xml");
}
@Test
public void testFailure_clientTransferProhibited() throws Exception {
setupDomain("example", "tld");
domain = persistResource(
domain.asBuilder().addStatusValue(StatusValue.CLIENT_TRANSFER_PROHIBITED).build());
thrown.expect(ResourceStatusProhibitsOperationException.class, "clientTransferProhibited");
doFailingTest("domain_transfer_request.xml");
}
@Test
public void testFailure_serverTransferProhibited() throws Exception {
setupDomain("example", "tld");
domain = persistResource(
domain.asBuilder().addStatusValue(StatusValue.SERVER_TRANSFER_PROHIBITED).build());
thrown.expect(ResourceStatusProhibitsOperationException.class, "serverTransferProhibited");
doFailingTest("domain_transfer_request.xml");
}
@Test
public void testFailure_pendingDelete() throws Exception {
setupDomain("example", "tld");
domain = persistResource(
domain.asBuilder().addStatusValue(StatusValue.PENDING_DELETE).build());
thrown.expect(ResourceStatusProhibitsOperationException.class);
thrown.expect(ResourceStatusProhibitsOperationException.class, "pendingDelete");
doFailingTest("domain_transfer_request.xml");
}
}

View file

@ -832,30 +832,6 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
runFlow();
}
@Test
public void testFailure_clientUpdateProhibited() throws Exception {
createTld("com");
setEppInput("domain_update_authinfo.xml");
persistReferencedEntities();
persistResource(
newDomainResource(getUniqueIdFromCommand()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.CLIENT_UPDATE_PROHIBITED))
.build());
thrown.expect(ResourceHasClientUpdateProhibitedException.class);
runFlow();
}
@Test
public void testFailure_serverUpdateProhibited() throws Exception {
persistReferencedEntities();
persistResource(
newDomainResource(getUniqueIdFromCommand()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.SERVER_UPDATE_PROHIBITED))
.build());
thrown.expect(ResourceStatusProhibitsOperationException.class);
runFlow();
}
@Test
public void testFailure_missingHost() throws Exception {
persistActiveHost("ns1.example.foo");
@ -898,7 +874,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
}
@Test
public void testFailure_clientProhibitedStatusValue() throws Exception {
public void testFailure_statusValueNotClientSettable() throws Exception {
setEppInput("domain_update_prohibited_status.xml");
persistReferencedEntities();
persistDomain();
@ -907,7 +883,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
}
@Test
public void testSuccess_superuserClientProhibitedStatusValue() throws Exception {
public void testSuccess_superuserStatusValueNotClientSettable() throws Exception {
setEppInput("domain_update_prohibited_status.xml");
persistReferencedEntities();
persistDomain();
@ -917,6 +893,30 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
readFile("domain_update_response.xml"));
}
@Test
public void testFailure_clientUpdateProhibited() throws Exception {
createTld("com");
setEppInput("domain_update_authinfo.xml");
persistReferencedEntities();
persistResource(
newDomainResource(getUniqueIdFromCommand()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.CLIENT_UPDATE_PROHIBITED))
.build());
thrown.expect(ResourceHasClientUpdateProhibitedException.class);
runFlow();
}
@Test
public void testFailure_serverUpdateProhibited() throws Exception {
persistReferencedEntities();
persistResource(
newDomainResource(getUniqueIdFromCommand()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.SERVER_UPDATE_PROHIBITED))
.build());
thrown.expect(ResourceStatusProhibitsOperationException.class, "serverUpdateProhibited");
runFlow();
}
@Test
public void testFailure_pendingDelete() throws Exception {
persistReferencedEntities();
@ -925,7 +925,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
.setDeletionTime(clock.nowUtc().plusDays(1))
.addStatusValue(StatusValue.PENDING_DELETE)
.build());
thrown.expect(ResourceStatusProhibitsOperationException.class);
thrown.expect(ResourceStatusProhibitsOperationException.class, "pendingDelete");
runFlow();
}

View file

@ -54,16 +54,6 @@ public class HostDeleteFlowTest extends ResourceFlowTestCase<HostDeleteFlow, Hos
setEppInput("host_delete.xml", ImmutableMap.of("HOSTNAME", "ns1.example.tld"));
}
private void doFailingStatusTest(StatusValue statusValue, Class<? extends Exception> exception)
throws Exception {
persistResource(
newHostResource("ns1.example.tld").asBuilder()
.setStatusValues(ImmutableSet.of(statusValue))
.build());
thrown.expect(exception);
runFlow();
}
@Test
public void testDryRun() throws Exception {
persistActiveHost("ns1.example.tld");
@ -99,6 +89,16 @@ public class HostDeleteFlowTest extends ResourceFlowTestCase<HostDeleteFlow, Hos
runFlow();
}
private void doFailingStatusTest(StatusValue statusValue, Class<? extends Exception> exception)
throws Exception {
persistResource(
newHostResource("ns1.example.tld").asBuilder()
.setStatusValues(ImmutableSet.of(statusValue))
.build());
thrown.expect(exception, statusValue.getXmlName());
runFlow();
}
@Test
public void testFailure_existedButWasClientDeleteProhibited() throws Exception {
doFailingStatusTest(

View file

@ -152,20 +152,6 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
return renamedHost;
}
@Test
public void testSuccess_removeClientUpdateProhibited() throws Exception {
setEppInput("host_update_remove_client_update_prohibited.xml");
persistActiveHost(oldHostName());
persistResource(
newHostResource(oldHostName()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.CLIENT_UPDATE_PROHIBITED))
.build());
clock.advanceOneMilli();
runFlow();
assertAboutEppResources().that(reloadResourceByForeignKey())
.doesNotHaveStatusValue(StatusValue.CLIENT_UPDATE_PROHIBITED);
}
@Test
public void testSuccess_rename_noOtherHostEverUsedTheOldName() throws Exception {
persistActiveHost(oldHostName());
@ -828,26 +814,6 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
runFlow();
}
@Test
public void testFailure_clientUpdateProhibited() throws Exception {
persistResource(
newHostResource(oldHostName()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.CLIENT_UPDATE_PROHIBITED))
.build());
thrown.expect(ResourceHasClientUpdateProhibitedException.class);
runFlow();
}
@Test
public void testFailure_serverUpdateProhibited() throws Exception {
persistResource(
newHostResource(oldHostName()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.SERVER_UPDATE_PROHIBITED))
.build());
thrown.expect(ResourceStatusProhibitsOperationException.class);
runFlow();
}
@Test
public void testFailure_subordinateNeedsIps() throws Exception {
setEppHostUpdateInput(
@ -952,7 +918,51 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
}
@Test
public void testFailure_clientProhibitedStatusValue() throws Exception {
public void testSuccess_clientUpdateProhibited_removed() throws Exception {
setEppInput("host_update_remove_client_update_prohibited.xml");
persistActiveHost(oldHostName());
persistResource(
newHostResource(oldHostName()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.CLIENT_UPDATE_PROHIBITED))
.build());
clock.advanceOneMilli();
runFlow();
assertAboutEppResources().that(reloadResourceByForeignKey())
.doesNotHaveStatusValue(StatusValue.CLIENT_UPDATE_PROHIBITED);
}
@Test
public void testFailure_clientUpdateProhibited() throws Exception {
persistResource(
newHostResource(oldHostName()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.CLIENT_UPDATE_PROHIBITED))
.build());
thrown.expect(ResourceHasClientUpdateProhibitedException.class);
runFlow();
}
@Test
public void testFailure_serverUpdateProhibited() throws Exception {
persistResource(
newHostResource(oldHostName()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.SERVER_UPDATE_PROHIBITED))
.build());
thrown.expect(ResourceStatusProhibitsOperationException.class, "serverUpdateProhibited");
runFlow();
}
@Test
public void testFailure_pendingDelete() throws Exception {
persistResource(
newHostResource(oldHostName()).asBuilder()
.setStatusValues(ImmutableSet.of(StatusValue.PENDING_DELETE))
.build());
thrown.expect(ResourceStatusProhibitsOperationException.class, "pendingDelete");
runFlow();
}
@Test
public void testFailure_statusValueNotClientSettable() throws Exception {
createTld("tld");
persistActiveDomain("example.tld");
setEppInput("host_update_prohibited_status.xml");
@ -962,7 +972,7 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
}
@Test
public void testSuccess_superuserClientProhibitedStatusValue() throws Exception {
public void testSuccess_superuserStatusValueNotClientSettable() throws Exception {
setEppInput("host_update_prohibited_status.xml");
createTld("tld");
persistActiveDomain("example.tld");