Flatten the domain and domain application update flows

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=135282371

docs/flows.md

@@ -209,7 +209,20 @@ This flows can check the existence of multiple contacts simultaneously.
 
 ### Description
 
-An EPP flow that updates a domain resource.
+An EPP flow that updates a domain.
+
+Updates can change contacts, nameservers and delegation signer data of a domain. Updates
+cannot change the domain's name.
+
+Some status values (those of the form "serverSomethingProhibited") can only be applied by the
+superuser. As such, adding or removing these statuses incurs a billing event. There will be only
+one charge per update, even if several such statuses are updated at once.
+
+If a domain was created during the sunrise or landrush phases of a TLD, is still within the
+sunrushAddGracePeriod and has not yet been delegated in DNS, then it will not yet have been
+billed for. Any update that causes the name to be delegated (such * as adding nameservers or
+removing a hold status) will cause the domain to convert to a normal create and be billed for
+accordingly.
 
 ### Errors
 
@@ -227,9 +240,9 @@ An EPP flow that updates a domain resource.
 *   2103
     *   Specified extension is not implemented.
 *   2201
+    *   The specified resource belongs to another client.
     *   Only a tool can pass a metadata extension.
     *   Registrar is not authorized to access this TLD.
-    *   The specified resource belongs to another client.
 *   2303
     *   Resource with this id does not exist.
     *   Resource linked to this domain does not exist.
@@ -242,9 +255,9 @@ An EPP flow that updates a domain resource.
     *   Registrant is not whitelisted for this TLD.
 *   2306
     *   Cannot add and remove the same value.
-    *   The secDNS:all element must have value 'true' if present.
     *   More than one contact for a given role is not allowed.
     *   Missing type attribute for contact.
+    *   The secDNS:all element must have value 'true' if present.
     *   Too many DS records set on a domain.
     *   Too many nameservers set on this domain.
 
@@ -639,7 +652,10 @@ This flow also supports the EPP fee extension and can return pricing information
 
 ### Description
 
-An EPP flow that updates a domain resource.
+An EPP flow that updates a domain application.
+
+Updates can change contacts, nameservers and delegation signer data of an application. Updates
+cannot change the domain name that is being applied for.
 
 ### Errors
 
@@ -655,8 +671,8 @@ An EPP flow that updates a domain resource.
 *   2103
     *   Specified extension is not implemented.
 *   2201
-    *   Registrar is not authorized to access this TLD.
     *   The specified resource belongs to another client.
+    *   Registrar is not authorized to access this TLD.
 *   2303
     *   Resource with this id does not exist.
     *   Resource linked to this domain does not exist.
@@ -668,9 +684,9 @@ An EPP flow that updates a domain resource.
     *   Application status prohibits this domain update.
 *   2306
     *   Cannot add and remove the same value.
-    *   The secDNS:all element must have value 'true' if present.
     *   More than one contact for a given role is not allowed.
     *   Missing type attribute for contact.
+    *   The secDNS:all element must have value 'true' if present.
     *   Too many DS records set on a domain.
     *   Too many nameservers set on this domain.
 

java/google/registry/flows/domain/BaseDomainUpdateFlow.java

@@ -1,189 +0,0 @@
-// Copyright 2016 The Domain Registry Authors. All Rights Reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package google.registry.flows.domain;
-
-import static com.google.common.collect.Sets.difference;
-import static com.google.common.collect.Sets.union;
-import static google.registry.flows.domain.DomainFlowUtils.checkAllowedAccessToTld;
-import static google.registry.flows.domain.DomainFlowUtils.cloneAndLinkReferences;
-import static google.registry.flows.domain.DomainFlowUtils.validateContactsHaveTypes;
-import static google.registry.flows.domain.DomainFlowUtils.validateDsData;
-import static google.registry.flows.domain.DomainFlowUtils.validateNameserversAllowedOnTld;
-import static google.registry.flows.domain.DomainFlowUtils.validateNameserversCountForTld;
-import static google.registry.flows.domain.DomainFlowUtils.validateNoDuplicateContacts;
-import static google.registry.flows.domain.DomainFlowUtils.validateRegistrantAllowedOnTld;
-import static google.registry.flows.domain.DomainFlowUtils.validateRequiredContactsPresent;
-import static google.registry.flows.domain.DomainFlowUtils.verifyNotInPendingDelete;
-import static google.registry.model.domain.fee.Fee.FEE_UPDATE_COMMAND_EXTENSIONS_IN_PREFERENCE_ORDER;
-
-import com.google.common.base.Optional;
-import com.google.common.collect.ImmutableSet;
-import google.registry.flows.EppException;
-import google.registry.flows.EppException.ParameterValuePolicyErrorException;
-import google.registry.flows.EppException.RequiredParameterMissingException;
-import google.registry.flows.EppException.UnimplementedOptionException;
-import google.registry.flows.ResourceUpdateFlow;
-import google.registry.model.domain.DomainBase;
-import google.registry.model.domain.DomainBase.Builder;
-import google.registry.model.domain.DomainCommand.Update;
-import google.registry.model.domain.fee.FeeTransformCommandExtension;
-import google.registry.model.domain.secdns.DelegationSignerData;
-import google.registry.model.domain.secdns.SecDnsUpdateExtension;
-import google.registry.model.domain.secdns.SecDnsUpdateExtension.Add;
-import google.registry.model.domain.secdns.SecDnsUpdateExtension.Remove;
-import java.util.Set;
-import javax.annotation.Nullable;
-
-/**
- * An EPP flow that updates a domain application or resource.
- *
- * @param <R> the resource type being created
- * @param <B> a builder for the resource
- */
-public abstract class BaseDomainUpdateFlow<R extends DomainBase, B extends Builder<R, B>>
-    extends ResourceUpdateFlow<R, B, Update> {
-
-  @Nullable
-  protected FeeTransformCommandExtension feeUpdate;
-
-  protected Optional<RegistryExtraFlowLogic> extraFlowLogic;
-
-  @Override
-  public final void initResourceCreateOrMutateFlow() throws EppException {
-    registerExtensions(FEE_UPDATE_COMMAND_EXTENSIONS_IN_PREFERENCE_ORDER);
-    feeUpdate =
-        eppInput.getFirstExtensionOfClasses(FEE_UPDATE_COMMAND_EXTENSIONS_IN_PREFERENCE_ORDER);
-    command = cloneAndLinkReferences(command, now);
-    initDomainUpdateFlow();
-    extraFlowLogic = RegistryExtraFlowLogicProxy.newInstanceForDomain(existingResource);
-  }
-
-  @SuppressWarnings("unused")
-  protected void initDomainUpdateFlow() throws EppException {}
-
-  @Override
-  public final B setUpdateProperties(B builder) throws EppException {
-    // Handle the secDNS extension.
-    SecDnsUpdateExtension secDnsUpdate = eppInput.getSingleExtension(SecDnsUpdateExtension.class);
-    if (secDnsUpdate != null) {
-      // We don't support 'urgent' because we do everything as fast as we can anyways.
-      if (Boolean.TRUE.equals(secDnsUpdate.getUrgent())) {  // We allow both false and null.
-        throw new UrgentAttributeNotSupportedException();
-      }
-      // There must be at least one of add/rem/chg, and chg isn't actually supported.
-      if (secDnsUpdate.getAdd() == null && secDnsUpdate.getRemove() == null) {
-        // The only thing you can change is maxSigLife, and we don't support that at all.
-        throw (secDnsUpdate.getChange() == null)
-            ? new EmptySecDnsUpdateException()
-            : new MaxSigLifeChangeNotSupportedException();
-      }
-      Set<DelegationSignerData> newDsData = existingResource.getDsData();
-      // RFC 5910 specifies that removes are processed before adds.
-      Remove remove = secDnsUpdate.getRemove();
-      if (remove != null) {
-        if (Boolean.FALSE.equals(remove.getAll())) {  // Explicit all=false is meaningless.
-          throw new SecDnsAllUsageException();
-        }
-        newDsData = (remove.getAll() == null)
-            ? difference(existingResource.getDsData(), remove.getDsData())
-            : ImmutableSet.<DelegationSignerData>of();
-      }
-      Add add = secDnsUpdate.getAdd();
-      if (add != null) {
-        newDsData = union(newDsData, add.getDsData());
-      }
-      builder.setDsData(ImmutableSet.copyOf(newDsData));
-    }
-    return setDomainUpdateProperties(builder);
-  }
-
-  /**
-   * Subclasses can override this to do set more specific properties.
-   *
-   * @throws EppException if the overriding method encounters an error that should be returned to
-   *         the user as an EPP response
-   */
-  protected B setDomainUpdateProperties(B builder) throws EppException {
-    return builder;
-  }
-
-  @Override
-  protected final void verifyUpdateIsAllowed() throws EppException {
-    checkAllowedAccessToTld(getAllowedTlds(), existingResource.getTld());
-    verifyDomainUpdateIsAllowed();
-    verifyNotInPendingDelete(
-        command.getInnerAdd().getContacts(),
-        command.getInnerChange().getRegistrant(),
-        command.getInnerAdd().getNameservers());
-    validateContactsHaveTypes(command.getInnerAdd().getContacts());
-    validateContactsHaveTypes(command.getInnerRemove().getContacts());
-    validateRegistrantAllowedOnTld(
-        existingResource.getTld(), command.getInnerChange().getRegistrantContactId());
-    validateNameserversAllowedOnTld(
-        existingResource.getTld(), command.getInnerAdd().getNameserverFullyQualifiedHostNames());
-  }
-
-  /** Subclasses can override this to do more specific verification. */
-  @SuppressWarnings("unused")
-  protected void verifyDomainUpdateIsAllowed() throws EppException {}
-
-  @Override
-  protected final void verifyNewUpdatedStateIsAllowed() throws EppException {
-    validateNoDuplicateContacts(newResource.getContacts());
-    validateRequiredContactsPresent(newResource.getRegistrant(), newResource.getContacts());
-    validateDsData(newResource.getDsData());
-    validateNameserversCountForTld(newResource.getTld(), newResource.getNameservers().size());
-  }
-
-  /** Call the subclass method, then commit any extra flow logic. */
-  @Override
-  protected final void modifyRelatedResources() {
-    modifyUpdateRelatedResources();
-    if (extraFlowLogic.isPresent()) {
-      extraFlowLogic.get().commitAdditionalLogicChanges();
-    }
-  }
-
-  /** Modify any other resources that need to be informed of this update. */
-  protected void modifyUpdateRelatedResources() {}
-
-  /** The secDNS:all element must have value 'true' if present. */
-  static class SecDnsAllUsageException extends ParameterValuePolicyErrorException {
-    public SecDnsAllUsageException() {
-      super("The secDNS:all element must have value 'true' if present");
-    }
-  }
-
-  /** At least one of 'add' or 'rem' is required on a secDNS update. */
-  static class EmptySecDnsUpdateException extends RequiredParameterMissingException {
-    public EmptySecDnsUpdateException() {
-      super("At least one of 'add' or 'rem' is required on a secDNS update");
-    }
-  }
-
-  /** The 'urgent' attribute is not supported. */
-  static class UrgentAttributeNotSupportedException extends UnimplementedOptionException {
-    public UrgentAttributeNotSupportedException() {
-      super("The 'urgent' attribute is not supported");
-    }
-  }
-
-  /** Changing 'maxSigLife' is not supported. */
-  static class MaxSigLifeChangeNotSupportedException extends UnimplementedOptionException {
-    public MaxSigLifeChangeNotSupportedException() {
-      super("Changing 'maxSigLife' is not supported");
-    }
-  }
-}

java/google/registry/flows/domain/DomainApplicationUpdateFlow.java

@@ -16,77 +16,188 @@ package google.registry.flows.domain;
 
 import static com.google.common.base.CaseFormat.LOWER_CAMEL;
 import static com.google.common.base.CaseFormat.UPPER_UNDERSCORE;
-import static google.registry.flows.domain.DomainFlowUtils.DISALLOWED_TLD_STATES_FOR_LAUNCH_FLOWS;
+import static google.registry.flows.ResourceFlowUtils.verifyExistence;
+import static google.registry.flows.ResourceFlowUtils.verifyNoDisallowedStatuses;
+import static google.registry.flows.ResourceFlowUtils.verifyOptionalAuthInfoForResource;
+import static google.registry.flows.ResourceFlowUtils.verifyResourceOwnership;
+import static google.registry.flows.domain.DomainFlowUtils.checkAllowedAccessToTld;
+import static google.registry.flows.domain.DomainFlowUtils.cloneAndLinkReferences;
+import static google.registry.flows.domain.DomainFlowUtils.updateDsData;
+import static google.registry.flows.domain.DomainFlowUtils.validateContactsHaveTypes;
+import static google.registry.flows.domain.DomainFlowUtils.validateDsData;
+import static google.registry.flows.domain.DomainFlowUtils.validateNameserversAllowedOnTld;
+import static google.registry.flows.domain.DomainFlowUtils.validateNameserversCountForTld;
+import static google.registry.flows.domain.DomainFlowUtils.validateNoDuplicateContacts;
+import static google.registry.flows.domain.DomainFlowUtils.validateRegistrantAllowedOnTld;
+import static google.registry.flows.domain.DomainFlowUtils.validateRequiredContactsPresent;
+import static google.registry.flows.domain.DomainFlowUtils.verifyClientUpdateNotProhibited;
+import static google.registry.flows.domain.DomainFlowUtils.verifyNotInPendingDelete;
+import static google.registry.flows.domain.DomainFlowUtils.verifyStatusChangesAreClientSettable;
+import static google.registry.model.EppResourceUtils.loadDomainApplication;
+import static google.registry.model.domain.fee.Fee.FEE_UPDATE_COMMAND_EXTENSIONS_IN_PREFERENCE_ORDER;
+import static google.registry.model.eppoutput.Result.Code.SUCCESS;
+import static google.registry.model.ofy.ObjectifyService.ofy;
 
+import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
+import com.googlecode.objectify.Key;
 import google.registry.flows.EppException;
 import google.registry.flows.EppException.StatusProhibitsOperationException;
+import google.registry.flows.FlowModule.ApplicationId;
+import google.registry.flows.FlowModule.ClientId;
+import google.registry.flows.FlowModule.TargetId;
+import google.registry.flows.LoggedInFlow;
+import google.registry.flows.TransactionalFlow;
+import google.registry.flows.exceptions.AddRemoveSameValueEppException;
+import google.registry.model.ImmutableObject;
 import google.registry.model.domain.DomainApplication;
 import google.registry.model.domain.DomainApplication.Builder;
+import google.registry.model.domain.DomainCommand.Update;
 import google.registry.model.domain.launch.ApplicationStatus;
 import google.registry.model.domain.launch.LaunchUpdateExtension;
+import google.registry.model.domain.metadata.MetadataExtension;
 import google.registry.model.domain.secdns.SecDnsUpdateExtension;
-import google.registry.model.registry.Registry.TldState;
+import google.registry.model.eppcommon.AuthInfo;
+import google.registry.model.eppcommon.StatusValue;
+import google.registry.model.eppinput.ResourceCommand;
+import google.registry.model.eppinput.ResourceCommand.AddRemoveSameValueException;
+import google.registry.model.eppoutput.EppOutput;
 import google.registry.model.reporting.HistoryEntry;
 import javax.inject.Inject;
 
 /**
- * An EPP flow that updates a domain resource.
+ * An EPP flow that updates a domain application.
+ *
+ * <p>Updates can change contacts, nameservers and delegation signer data of an application. Updates
+ * cannot change the domain name that is being applied for.
  *
  * @error {@link google.registry.flows.EppException.UnimplementedExtensionException}
- * @error {@link google.registry.flows.domain.DomainFlowUtils.NotAuthorizedForTldException}
+ * @error {@link google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException}
  * @error {@link google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException}
- * @error {@link google.registry.flows.ResourceMutateFlow.ResourceDoesNotExistException}
+ * @error {@link google.registry.flows.exceptions.AddRemoveSameValueEppException}
- * @error {@link google.registry.flows.ResourceUpdateFlow.AddRemoveSameValueEppException}
+ * @error {@link google.registry.flows.exceptions.ResourceHasClientUpdateProhibitedException}
- * @error {@link google.registry.flows.ResourceUpdateFlow.ResourceHasClientUpdateProhibitedException}
+ * @error {@link google.registry.flows.exceptions.ResourceStatusProhibitsOperationException}
- * @error {@link google.registry.flows.ResourceUpdateFlow.StatusNotClientSettableException}
+ * @error {@link google.registry.flows.exceptions.StatusNotClientSettableException}
- * @error {@link google.registry.flows.SingleResourceFlow.ResourceStatusProhibitsOperationException}
- * @error {@link BaseDomainUpdateFlow.EmptySecDnsUpdateException}
- * @error {@link BaseDomainUpdateFlow.MaxSigLifeChangeNotSupportedException}
- * @error {@link BaseDomainUpdateFlow.SecDnsAllUsageException}
- * @error {@link BaseDomainUpdateFlow.UrgentAttributeNotSupportedException}
  * @error {@link DomainFlowUtils.DuplicateContactForRoleException}
+ * @error {@link DomainFlowUtils.EmptySecDnsUpdateException}
  * @error {@link DomainFlowUtils.LinkedResourcesDoNotExistException}
+ * @error {@link DomainFlowUtils.MaxSigLifeChangeNotSupportedException}
  * @error {@link DomainFlowUtils.MissingAdminContactException}
  * @error {@link DomainFlowUtils.MissingContactTypeException}
  * @error {@link DomainFlowUtils.MissingTechnicalContactException}
  * @error {@link DomainFlowUtils.NameserversNotAllowedException}
+ * @error {@link DomainFlowUtils.NotAuthorizedForTldException}
  * @error {@link DomainFlowUtils.RegistrantNotAllowedException}
+ * @error {@link DomainFlowUtils.SecDnsAllUsageException}
  * @error {@link DomainFlowUtils.TooManyDsRecordsException}
  * @error {@link DomainFlowUtils.TooManyNameserversException}
+ * @error {@link DomainFlowUtils.UrgentAttributeNotSupportedException}
  * @error {@link DomainApplicationUpdateFlow.ApplicationStatusProhibitsUpdateException}
  */
-public class DomainApplicationUpdateFlow
+public class DomainApplicationUpdateFlow extends LoggedInFlow implements TransactionalFlow {
-    extends BaseDomainUpdateFlow<DomainApplication, Builder> {
 
+  /**
+   * Note that CLIENT_UPDATE_PROHIBITED is intentionally not in this list. This is because it
+   * requires special checking, since you must be able to clear the status off the object with an
+   * update.
+   */
+  private static final ImmutableSet<StatusValue> UPDATE_DISALLOWED_STATUSES =
+      Sets.immutableEnumSet(
+          StatusValue.PENDING_DELETE,
+          StatusValue.SERVER_UPDATE_PROHIBITED);
+
+  private static final ImmutableSet<ApplicationStatus> UPDATE_DISALLOWED_APPLICATION_STATUSES =
+      Sets.immutableEnumSet(
+          ApplicationStatus.INVALID,
+          ApplicationStatus.REJECTED,
+          ApplicationStatus.ALLOCATED);
+
+  @Inject ResourceCommand resourceCommand;
+  @Inject Optional<AuthInfo> authInfo;
+  @Inject @ClientId String clientId;
+  @Inject @TargetId String targetId;
+  @Inject @ApplicationId String applicationId;
+  @Inject HistoryEntry.Builder historyBuilder;
   @Inject DomainApplicationUpdateFlow() {}
 
   @Override
-  protected void initDomainUpdateFlow() throws EppException {
+  protected final void initLoggedInFlow() throws EppException {
-    registerExtensions(LaunchUpdateExtension.class, SecDnsUpdateExtension.class);
+    registerExtensions(FEE_UPDATE_COMMAND_EXTENSIONS_IN_PREFERENCE_ORDER);
+    registerExtensions(
+        MetadataExtension.class, LaunchUpdateExtension.class, SecDnsUpdateExtension.class);
   }
 
   @Override
-  protected final void verifyDomainUpdateIsAllowed() throws EppException {
+  public final EppOutput run() throws EppException {
-    switch (existingResource.getApplicationStatus()) {
+    Update command = cloneAndLinkReferences((Update) resourceCommand, now);
-      case PENDING_ALLOCATION:
+    DomainApplication existingApplication = verifyExistence(
-      case PENDING_VALIDATION:
+        DomainApplication.class, applicationId, loadDomainApplication(applicationId, now));
-      case VALIDATED:
+    verifyNoDisallowedStatuses(existingApplication, UPDATE_DISALLOWED_STATUSES);
-        return;
+    verifyOptionalAuthInfoForResource(authInfo, existingApplication);
-      default:
+    verifyUpdateAllowed(existingApplication, command);
-        throw new ApplicationStatusProhibitsUpdateException(
+    HistoryEntry historyEntry = buildHistory(existingApplication);
-            existingResource.getApplicationStatus());
+    DomainApplication newApplication = updateApplication(existingApplication, command);
+    validateNewApplication(newApplication);
+    ofy().save().<ImmutableObject>entities(newApplication, historyEntry);
+    return createOutput(SUCCESS);
+  }
+
+  protected final void verifyUpdateAllowed(
+      DomainApplication existingApplication, Update command) throws EppException {
+    if (!isSuperuser) {
+      verifyResourceOwnership(clientId, existingApplication);
+      verifyClientUpdateNotProhibited(command, existingApplication);
+      verifyStatusChangesAreClientSettable(command);
     }
+    String tld = existingApplication.getTld();
+    checkAllowedAccessToTld(getAllowedTlds(), tld);
+    if (UPDATE_DISALLOWED_APPLICATION_STATUSES
+        .contains(existingApplication.getApplicationStatus())) {
+      throw new ApplicationStatusProhibitsUpdateException(
+          existingApplication.getApplicationStatus());
+    }
+    verifyNotInPendingDelete(
+        command.getInnerAdd().getContacts(),
+        command.getInnerChange().getRegistrant(),
+        command.getInnerAdd().getNameservers());
+    validateContactsHaveTypes(command.getInnerAdd().getContacts());
+    validateContactsHaveTypes(command.getInnerRemove().getContacts());
+    validateRegistrantAllowedOnTld(tld, command.getInnerChange().getRegistrantContactId());
+    validateNameserversAllowedOnTld(
+        tld, command.getInnerAdd().getNameserverFullyQualifiedHostNames());
   }
 
-  @Override
+  private HistoryEntry buildHistory(DomainApplication existingApplication) {
-  protected final ImmutableSet<TldState> getDisallowedTldStates() {
+    return historyBuilder
-    return DISALLOWED_TLD_STATES_FOR_LAUNCH_FLOWS;
+        .setType(HistoryEntry.Type.DOMAIN_APPLICATION_UPDATE)
+        .setModificationTime(now)
+        .setParent(Key.create(existingApplication))
+        .build();
   }
 
-  @Override
+  private DomainApplication updateApplication(
-  protected final HistoryEntry.Type getHistoryEntryType() {
+      DomainApplication existingApplication, Update command) throws EppException {
-    return HistoryEntry.Type.DOMAIN_APPLICATION_UPDATE;
+    Builder builder = existingApplication.asBuilder();
+    try {
+      command.applyTo(builder);
+    } catch (AddRemoveSameValueException e) {
+      throw new AddRemoveSameValueEppException();
+    }
+    builder.setLastEppUpdateTime(now).setLastEppUpdateClientId(clientId);
+    // Handle the secDNS extension.
+    SecDnsUpdateExtension secDnsUpdate = eppInput.getSingleExtension(SecDnsUpdateExtension.class);
+    if (secDnsUpdate != null) {
+      builder.setDsData(updateDsData(existingApplication.getDsData(), secDnsUpdate));
+    }
+    return builder.build();
+  }
+
+  private void validateNewApplication(DomainApplication newApplication) throws EppException {
+    validateNoDuplicateContacts(newApplication.getContacts());
+    validateRequiredContactsPresent(newApplication.getRegistrant(), newApplication.getContacts());
+    validateDsData(newApplication.getDsData());
+    validateNameserversCountForTld(newApplication.getTld(), newApplication.getNameservers().size());
   }
 
   /** Application status prohibits this domain update. */

java/google/registry/flows/domain/DomainFlowUtils.java

@@ -20,6 +20,7 @@ import static com.google.common.base.Predicates.equalTo;
 import static com.google.common.collect.Iterables.any;
 import static com.google.common.collect.Iterables.concat;
 import static com.google.common.collect.Sets.difference;
+import static com.google.common.collect.Sets.union;
 import static google.registry.flows.EppXmlTransformer.unmarshal;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.model.registry.Registries.findTldForName;
@@ -50,6 +51,8 @@ import google.registry.flows.EppException.ParameterValueSyntaxErrorException;
 import google.registry.flows.EppException.RequiredParameterMissingException;
 import google.registry.flows.EppException.StatusProhibitsOperationException;
 import google.registry.flows.EppException.UnimplementedOptionException;
+import google.registry.flows.exceptions.ResourceHasClientUpdateProhibitedException;
+import google.registry.flows.exceptions.StatusNotClientSettableException;
 import google.registry.model.EppResource;
 import google.registry.model.billing.BillingEvent;
 import google.registry.model.billing.BillingEvent.Flag;
@@ -61,6 +64,7 @@ import google.registry.model.domain.DomainApplication;
 import google.registry.model.domain.DomainBase;
 import google.registry.model.domain.DomainCommand.CreateOrUpdate;
 import google.registry.model.domain.DomainCommand.InvalidReferencesException;
+import google.registry.model.domain.DomainCommand.Update;
 import google.registry.model.domain.DomainResource;
 import google.registry.model.domain.Period;
 import google.registry.model.domain.fee.Credit;
@@ -72,9 +76,11 @@ import google.registry.model.domain.launch.LaunchExtension;
 import google.registry.model.domain.launch.LaunchPhase;
 import google.registry.model.domain.secdns.DelegationSignerData;
 import google.registry.model.domain.secdns.SecDnsInfoExtension;
+import google.registry.model.domain.secdns.SecDnsUpdateExtension;
+import google.registry.model.domain.secdns.SecDnsUpdateExtension.Add;
+import google.registry.model.domain.secdns.SecDnsUpdateExtension.Remove;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.eppinput.EppInput;
-import google.registry.model.eppinput.ResourceCommand.SingleResourceCommand;
 import google.registry.model.eppoutput.EppResponse.ResponseExtension;
 import google.registry.model.host.HostResource;
 import google.registry.model.mark.Mark;
@@ -376,17 +382,6 @@ public class DomainFlowUtils {
     }
   }
 
-  /**
-   * Verifies that a launch extension's application id refers to an application with the same
-   * domain name as the one specified in the launch command.
-   */
-  static void verifyLaunchApplicationIdMatchesDomain(
-      SingleResourceCommand command, DomainBase existingResource) throws EppException {
-    if (!Objects.equals(command.getTargetId(), existingResource.getFullyQualifiedDomainName())) {
-      throw new ApplicationDomainNameMismatchException();
-    }
-  }
-
   /** Verifies that an application's domain name matches the target id (from a command). */
   static void verifyApplicationDomainMatchesTargetId(
       DomainApplication application, String targetId) throws EppException {
@@ -488,7 +483,7 @@ public class DomainFlowUtils {
         .build());
   }
 
-  public static SignedMark verifySignedMarks(
+  static SignedMark verifySignedMarks(
       ImmutableList<AbstractSignedMark> signedMarks, String domainLabel, DateTime now)
           throws EppException {
     if (signedMarks.size() > 1) {
@@ -787,6 +782,59 @@ public class DomainFlowUtils {
     }
   }
 
+  /** Update {@link DelegationSignerData} based on an update extension command. */
+  static ImmutableSet<DelegationSignerData> updateDsData(
+      ImmutableSet<DelegationSignerData> oldDsData, SecDnsUpdateExtension secDnsUpdate)
+          throws EppException {
+    // We don't support 'urgent' because we do everything as fast as we can anyways.
+    if (Boolean.TRUE.equals(secDnsUpdate.getUrgent())) {  // We allow both false and null.
+      throw new UrgentAttributeNotSupportedException();
+    }
+    // There must be at least one of add/rem/chg, and chg isn't actually supported.
+    if (secDnsUpdate.getChange() != null) {
+      // The only thing you can change is maxSigLife, and we don't support that at all.
+      throw new MaxSigLifeChangeNotSupportedException();
+    }
+    Add add = secDnsUpdate.getAdd();
+    Remove remove = secDnsUpdate.getRemove();
+    if (add == null && remove == null) {
+      throw new EmptySecDnsUpdateException();
+    }
+    if (remove != null && Boolean.FALSE.equals(remove.getAll())) {
+      throw new SecDnsAllUsageException();  // Explicit all=false is meaningless.
+    }
+    Set<DelegationSignerData> toAdd = (add == null)
+        ? ImmutableSet.<DelegationSignerData>of()
+        : add.getDsData();
+    Set<DelegationSignerData> toRemove = (remove == null)
+        ? ImmutableSet.<DelegationSignerData>of()
+        : (remove.getAll() == null) ? remove.getDsData() : oldDsData;
+    // RFC 5910 specifies that removes are processed before adds.
+    return ImmutableSet.copyOf(union(difference(oldDsData, toRemove), toAdd));
+  }
+
+  /** Check that all of the status values added or removed in an update are client-settable. */
+  static void verifyStatusChangesAreClientSettable(Update command)
+      throws StatusNotClientSettableException {
+    for (StatusValue statusValue : union(
+        command.getInnerAdd().getStatusValues(),
+        command.getInnerRemove().getStatusValues())) {
+      if (!statusValue.isClientSettable()) {
+        throw new StatusNotClientSettableException(statusValue.getXmlName());
+      }
+    }
+  }
+
+  /** If a domain or application has "clientUpdateProhibited" set, updates must clear it or fail. */
+  public static void verifyClientUpdateNotProhibited(Update command, DomainBase existingResource)
+      throws ResourceHasClientUpdateProhibitedException {
+    if (existingResource.getStatusValues().contains(StatusValue.CLIENT_UPDATE_PROHIBITED)
+        && !command.getInnerRemove().getStatusValues()
+            .contains(StatusValue.CLIENT_UPDATE_PROHIBITED)) {
+      throw new ResourceHasClientUpdateProhibitedException();
+    }
+  }
+
   /** Encoded signed marks must use base64 encoding. */
   static class Base64RequiredForEncodedSignedMarksException
       extends ParameterValuePolicyErrorException {
@@ -1156,4 +1204,31 @@ public class DomainFlowUtils {
     }
   }
 
+  /** The secDNS:all element must have value 'true' if present. */
+  static class SecDnsAllUsageException extends ParameterValuePolicyErrorException {
+    public SecDnsAllUsageException() {
+      super("The secDNS:all element must have value 'true' if present");
+    }
+  }
+
+  /** At least one of 'add' or 'rem' is required on a secDNS update. */
+  static class EmptySecDnsUpdateException extends RequiredParameterMissingException {
+    public EmptySecDnsUpdateException() {
+      super("At least one of 'add' or 'rem' is required on a secDNS update");
+    }
+  }
+
+  /** The 'urgent' attribute is not supported. */
+  static class UrgentAttributeNotSupportedException extends UnimplementedOptionException {
+    public UrgentAttributeNotSupportedException() {
+      super("The 'urgent' attribute is not supported");
+    }
+  }
+
+  /** Changing 'maxSigLife' is not supported. */
+  static class MaxSigLifeChangeNotSupportedException extends UnimplementedOptionException {
+    public MaxSigLifeChangeNotSupportedException() {
+      super("Changing 'maxSigLife' is not supported");
+    }
+  }
 }

java/google/registry/flows/domain/DomainUpdateFlow.java

@@ -15,188 +15,307 @@
 package google.registry.flows.domain;
 
 import static com.google.common.collect.Sets.symmetricDifference;
+import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence;
+import static google.registry.flows.ResourceFlowUtils.verifyNoDisallowedStatuses;
+import static google.registry.flows.ResourceFlowUtils.verifyOptionalAuthInfoForResource;
+import static google.registry.flows.ResourceFlowUtils.verifyResourceOwnership;
+import static google.registry.flows.domain.DomainFlowUtils.checkAllowedAccessToTld;
+import static google.registry.flows.domain.DomainFlowUtils.cloneAndLinkReferences;
+import static google.registry.flows.domain.DomainFlowUtils.updateDsData;
+import static google.registry.flows.domain.DomainFlowUtils.validateContactsHaveTypes;
+import static google.registry.flows.domain.DomainFlowUtils.validateDsData;
 import static google.registry.flows.domain.DomainFlowUtils.validateFeeChallenge;
+import static google.registry.flows.domain.DomainFlowUtils.validateNameserversAllowedOnTld;
+import static google.registry.flows.domain.DomainFlowUtils.validateNameserversCountForTld;
+import static google.registry.flows.domain.DomainFlowUtils.validateNoDuplicateContacts;
+import static google.registry.flows.domain.DomainFlowUtils.validateRegistrantAllowedOnTld;
+import static google.registry.flows.domain.DomainFlowUtils.validateRequiredContactsPresent;
+import static google.registry.flows.domain.DomainFlowUtils.verifyClientUpdateNotProhibited;
+import static google.registry.flows.domain.DomainFlowUtils.verifyNotInPendingDelete;
+import static google.registry.flows.domain.DomainFlowUtils.verifyStatusChangesAreClientSettable;
+import static google.registry.model.domain.fee.Fee.FEE_UPDATE_COMMAND_EXTENSIONS_IN_PREFERENCE_ORDER;
+import static google.registry.model.eppoutput.Result.Code.SUCCESS;
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.util.DateTimeUtils.earliestOf;
 
 import com.google.common.base.Optional;
-import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Iterables;
+import com.google.common.collect.ImmutableSet;
+import com.googlecode.objectify.Key;
 import google.registry.dns.DnsQueue;
 import google.registry.flows.EppException;
+import google.registry.flows.FlowModule.ClientId;
+import google.registry.flows.FlowModule.TargetId;
+import google.registry.flows.LoggedInFlow;
+import google.registry.flows.TransactionalFlow;
 import google.registry.flows.domain.DomainFlowUtils.FeesRequiredForNonFreeUpdateException;
 import google.registry.flows.domain.TldSpecificLogicProxy.EppCommandOperations;
+import google.registry.flows.exceptions.AddRemoveSameValueEppException;
+import google.registry.model.ImmutableObject;
 import google.registry.model.billing.BillingEvent;
 import google.registry.model.billing.BillingEvent.Reason;
+import google.registry.model.domain.DomainCommand.Update;
 import google.registry.model.domain.DomainResource;
-import google.registry.model.domain.DomainResource.Builder;
 import google.registry.model.domain.GracePeriod;
+import google.registry.model.domain.fee.FeeTransformCommandExtension;
 import google.registry.model.domain.flags.FlagsUpdateCommandExtension;
+import google.registry.model.domain.metadata.MetadataExtension;
 import google.registry.model.domain.rgp.GracePeriodStatus;
 import google.registry.model.domain.secdns.SecDnsUpdateExtension;
+import google.registry.model.eppcommon.AuthInfo;
 import google.registry.model.eppcommon.StatusValue;
+import google.registry.model.eppinput.ResourceCommand;
+import google.registry.model.eppinput.ResourceCommand.AddRemoveSameValueException;
+import google.registry.model.eppoutput.EppOutput;
 import google.registry.model.registry.Registry;
 import google.registry.model.reporting.HistoryEntry;
-import java.util.Set;
 import javax.inject.Inject;
 import org.joda.money.Money;
 import org.joda.time.DateTime;
 
 /**
- * An EPP flow that updates a domain resource.
+ * An EPP flow that updates a domain.
+ *
+ * <p>Updates can change contacts, nameservers and delegation signer data of a domain. Updates
+ * cannot change the domain's name.
+ *
+ * <p>Some status values (those of the form "serverSomethingProhibited") can only be applied by the
+ * superuser. As such, adding or removing these statuses incurs a billing event. There will be only
+ * one charge per update, even if several such statuses are updated at once.
+ *
+ * <p>If a domain was created during the sunrise or landrush phases of a TLD, is still within the
+ * sunrushAddGracePeriod and has not yet been delegated in DNS, then it will not yet have been
+ * billed for. Any update that causes the name to be delegated (such * as adding nameservers or
+ * removing a hold status) will cause the domain to convert to a normal create and be billed for
+ * accordingly.
  *
  * @error {@link google.registry.flows.EppException.UnimplementedExtensionException}
- * @error {@link google.registry.flows.ResourceCreateOrMutateFlow.OnlyToolCanPassMetadataException}
+ * @error {@link google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException}
- * @error {@link google.registry.flows.domain.DomainFlowUtils.NotAuthorizedForTldException}
  * @error {@link google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException}
- * @error {@link google.registry.flows.ResourceMutateFlow.ResourceDoesNotExistException}
+ * @error {@link google.registry.flows.exceptions.AddRemoveSameValueEppException}
- * @error {@link google.registry.flows.ResourceUpdateFlow.AddRemoveSameValueEppException}
+ * @error {@link google.registry.flows.exceptions.OnlyToolCanPassMetadataException}
- * @error {@link google.registry.flows.ResourceUpdateFlow.ResourceHasClientUpdateProhibitedException}
+ * @error {@link google.registry.flows.exceptions.ResourceHasClientUpdateProhibitedException}
- * @error {@link google.registry.flows.ResourceUpdateFlow.StatusNotClientSettableException}
+ * @error {@link google.registry.flows.exceptions.ResourceStatusProhibitsOperationException}
- * @error {@link google.registry.flows.SingleResourceFlow.ResourceStatusProhibitsOperationException}
+ * @error {@link google.registry.flows.exceptions.StatusNotClientSettableException}
- * @error {@link BaseDomainUpdateFlow.EmptySecDnsUpdateException}
- * @error {@link BaseDomainUpdateFlow.MaxSigLifeChangeNotSupportedException}
- * @error {@link BaseDomainUpdateFlow.SecDnsAllUsageException}
- * @error {@link BaseDomainUpdateFlow.UrgentAttributeNotSupportedException}
  * @error {@link DomainFlowUtils.DuplicateContactForRoleException}
+ * @error {@link DomainFlowUtils.EmptySecDnsUpdateException}
  * @error {@link DomainFlowUtils.FeesMismatchException}
  * @error {@link DomainFlowUtils.FeesRequiredForNonFreeUpdateException}
  * @error {@link DomainFlowUtils.LinkedResourcesDoNotExistException}
  * @error {@link DomainFlowUtils.LinkedResourceInPendingDeleteProhibitsOperationException}
+ * @error {@link DomainFlowUtils.MaxSigLifeChangeNotSupportedException}
  * @error {@link DomainFlowUtils.MissingAdminContactException}
  * @error {@link DomainFlowUtils.MissingContactTypeException}
  * @error {@link DomainFlowUtils.MissingTechnicalContactException}
  * @error {@link DomainFlowUtils.NameserversNotAllowedException}
  * @error {@link DomainFlowUtils.NameserversNotSpecifiedException}
+ * @error {@link DomainFlowUtils.NotAuthorizedForTldException}
  * @error {@link DomainFlowUtils.RegistrantNotAllowedException}
+ * @error {@link DomainFlowUtils.SecDnsAllUsageException}
  * @error {@link DomainFlowUtils.TooManyDsRecordsException}
  * @error {@link DomainFlowUtils.TooManyNameserversException}
+ * @error {@link DomainFlowUtils.UrgentAttributeNotSupportedException}
  */
-public class DomainUpdateFlow extends BaseDomainUpdateFlow<DomainResource, Builder> {
+public final class DomainUpdateFlow extends LoggedInFlow implements TransactionalFlow {
 
+  /**
+   * Note that CLIENT_UPDATE_PROHIBITED is intentionally not in this list. This is because it
+   * requires special checking, since you must be able to clear the status off the object with an
+   * update.
+   */
+  private static final ImmutableSet<StatusValue> UPDATE_DISALLOWED_STATUSES = ImmutableSet.of(
+      StatusValue.PENDING_DELETE,
+      StatusValue.SERVER_UPDATE_PROHIBITED);
+
+  @Inject ResourceCommand resourceCommand;
+  @Inject Optional<AuthInfo> authInfo;
+  @Inject @ClientId String clientId;
+  @Inject @TargetId String targetId;
+  @Inject HistoryEntry.Builder historyBuilder;
   @Inject DomainUpdateFlow() {}
 
   @Override
-  protected void initDomainUpdateFlow() {
+  protected final void initLoggedInFlow() throws EppException {
-    registerExtensions(SecDnsUpdateExtension.class, FlagsUpdateCommandExtension.class);
+    registerExtensions(FEE_UPDATE_COMMAND_EXTENSIONS_IN_PREFERENCE_ORDER);
+    registerExtensions(
+        MetadataExtension.class, SecDnsUpdateExtension.class, FlagsUpdateCommandExtension.class);
   }
 
   @Override
-  protected Builder setDomainUpdateProperties(Builder builder) throws EppException {
+  public EppOutput run() throws EppException {
-    // Check if the domain is currently in the sunrush add grace period.
+    Update command = cloneAndLinkReferences((Update) resourceCommand, now);
-    Optional<GracePeriod> sunrushAddGracePeriod = Iterables.tryFind(
+    DomainResource existingDomain = loadAndVerifyExistence(DomainResource.class, targetId, now);
-        existingResource.getGracePeriods(),
+    verifyUpdateAllowed(command, existingDomain);
-        new Predicate<GracePeriod>() {
+    HistoryEntry historyEntry = buildHistoryEntry(existingDomain);
-          @Override
+    DomainResource newDomain = performUpdate(command, existingDomain);
-          public boolean apply(GracePeriod gracePeriod) {
+    // If the new domain is in the sunrush add grace period and is now publishable to DNS because we
-            return gracePeriod.isSunrushAddGracePeriod();
+    // have added nameserver or removed holds, we have to convert it to a standard add grace period.
-          }});
+    if (newDomain.shouldPublishToDns()) {
-
+      for (GracePeriod gracePeriod : newDomain.getGracePeriods()) {
-    // If this domain is currently in the sunrush add grace period, and we're updating it in a way
+        if (gracePeriod.isSunrushAddGracePeriod()) {
-    // that will cause it to now get delegated (either by setting nameservers, or by removing a
+          newDomain = convertSunrushAddToAdd(newDomain, gracePeriod, historyEntry);
-    // clientHold or serverHold), then that will remove the sunrush add grace period and convert
+          break;  // There can only be one sunrush add grace period.
-    // that to a standard add grace period.
+        }
-    DomainResource updatedDomain = builder.build();
+      }
-    builder = updatedDomain.asBuilder();
-    if (sunrushAddGracePeriod.isPresent() && updatedDomain.shouldPublishToDns()) {
-      // Remove the sunrush grace period and write a billing event cancellation for it.
-      builder.removeGracePeriod(sunrushAddGracePeriod.get());
-      BillingEvent.Cancellation billingEventCancellation = BillingEvent.Cancellation
-          .forGracePeriod(sunrushAddGracePeriod.get(), historyEntry, targetId);
-
-      // Compute the expiration time of the add grace period. We will not allow it to be after the
-      // sunrush add grace period expiration time (i.e. you can't get extra add grace period by
-      // setting a nameserver).
-      DateTime addGracePeriodExpirationTime = earliestOf(
-          now.plus(Registry.get(existingResource.getTld()).getAddGracePeriodLength()),
-          sunrushAddGracePeriod.get().getExpirationTime());
-
-      // Create a new billing event for the add grace period. Note that we do this even if it would
-      // occur at the same time as the sunrush add grace period, as the event time will differ
-      // between them.
-      BillingEvent.OneTime originalAddEvent =
-          ofy().load().key(sunrushAddGracePeriod.get().getOneTimeBillingEvent()).now();
-      BillingEvent.OneTime billingEvent = new BillingEvent.OneTime.Builder()
-          .setReason(Reason.CREATE)
-          .setTargetId(targetId)
-          .setFlags(originalAddEvent.getFlags())
-          .setClientId(sunrushAddGracePeriod.get().getClientId())
-          .setCost(originalAddEvent.getCost())
-          .setPeriodYears(originalAddEvent.getPeriodYears())
-          .setEventTime(now)
-          .setBillingTime(addGracePeriodExpirationTime)
-          .setParent(historyEntry)
-          .build();
-
-      // Set the add grace period on the domain.
-      builder.addGracePeriod(GracePeriod.forBillingEvent(GracePeriodStatus.ADD, billingEvent));
-
-      // Save the billing events.
-      ofy().save().entities(billingEvent, billingEventCancellation);
     }
-
+    validateNewState(newDomain);
-    // Handle extra flow logic, if any.
+    DnsQueue.create().addDomainRefreshTask(targetId);
-    if (extraFlowLogic.isPresent()) {
+    handleExtraFlowLogic(existingDomain, historyEntry);
-      extraFlowLogic.get().performAdditionalDomainUpdateLogic(
+    ImmutableList.Builder<ImmutableObject> entitiesToSave = new ImmutableList.Builder<>();
-          existingResource, getClientId(), now, eppInput, historyEntry);
+    entitiesToSave.add(newDomain, historyEntry);
+    Optional<BillingEvent.OneTime> statusUpdateBillingEvent =
+        createBillingEventForStatusUpdates(existingDomain, newDomain, historyEntry);
+    if (statusUpdateBillingEvent.isPresent()) {
+      entitiesToSave.add(statusUpdateBillingEvent.get());
     }
-    return builder;
+    ofy().save().entities(entitiesToSave.build());
+    return createOutput(SUCCESS);
   }
 
-  @Override
+  /** Fail if the object doesn't exist or was deleted. */
-  protected final void verifyDomainUpdateIsAllowed() throws EppException {
+  private void verifyUpdateAllowed(Update command, DomainResource existingDomain)
+      throws EppException {
+    verifyNoDisallowedStatuses(existingDomain, UPDATE_DISALLOWED_STATUSES);
+    verifyOptionalAuthInfoForResource(authInfo, existingDomain);
+    if (!isSuperuser) {
+      verifyResourceOwnership(clientId, existingDomain);
+      verifyClientUpdateNotProhibited(command, existingDomain);
+      verifyStatusChangesAreClientSettable(command);
+    }
+    String tld = existingDomain.getTld();
+    checkAllowedAccessToTld(getAllowedTlds(), tld);
     EppCommandOperations commandOperations = TldSpecificLogicProxy.getUpdatePrice(
-        Registry.get(existingResource.getTld()),
+        Registry.get(tld), targetId, clientId, now, eppInput);
-        existingResource.getFullyQualifiedDomainName(),
-        getClientId(),
-        now,
-        eppInput);
 
+    FeeTransformCommandExtension feeUpdate =
+        eppInput.getFirstExtensionOfClasses(FEE_UPDATE_COMMAND_EXTENSIONS_IN_PREFERENCE_ORDER);
     // If the fee extension is present, validate it (even if the cost is zero, to check for price
     // mismatches). Don't rely on the the validateFeeChallenge check for feeUpdate nullness, because
     // it throws an error if the name is premium, and we don't want to do that here.
     Money totalCost = commandOperations.getTotalCost();
     if (feeUpdate != null) {
-      validateFeeChallenge(targetId, existingResource.getTld(), now, feeUpdate, totalCost);
+      validateFeeChallenge(targetId, existingDomain.getTld(), now, feeUpdate, totalCost);
-    // If it's not present but the cost is not zero, throw an exception.
     } else if (!totalCost.isZero()) {
+      // If it's not present but the cost is not zero, throw an exception.
       throw new FeesRequiredForNonFreeUpdateException();
     }
+    verifyNotInPendingDelete(
+        command.getInnerAdd().getContacts(),
+        command.getInnerChange().getRegistrant(),
+        command.getInnerAdd().getNameservers());
+    validateContactsHaveTypes(command.getInnerAdd().getContacts());
+    validateContactsHaveTypes(command.getInnerRemove().getContacts());
+    validateRegistrantAllowedOnTld(tld, command.getInnerChange().getRegistrantContactId());
+    validateNameserversAllowedOnTld(
+        tld, command.getInnerAdd().getNameserverFullyQualifiedHostNames());
   }
 
-  @Override
+  private HistoryEntry buildHistoryEntry(DomainResource existingDomain) {
-  protected final void modifyUpdateRelatedResources() {
+    return historyBuilder
-    // Determine the status changes, and filter to server statuses.
+        .setType(HistoryEntry.Type.DOMAIN_UPDATE)
-    // If any of these statuses have been added or removed, bill once.
+        .setModificationTime(now)
+        .setParent(Key.create(existingDomain))
+        .build();
+  }
+
+  private DomainResource performUpdate(Update command, DomainResource existingDomain)
+      throws EppException {
+    DomainResource.Builder builder = existingDomain.asBuilder()
+        .setLastEppUpdateTime(now)
+        .setLastEppUpdateClientId(clientId);
+    try {
+      command.applyTo(builder);
+    } catch (AddRemoveSameValueException e) {
+      throw new AddRemoveSameValueEppException();
+    }
+    // Handle the secDNS extension.
+    SecDnsUpdateExtension secDnsUpdate = eppInput.getSingleExtension(SecDnsUpdateExtension.class);
+    if (secDnsUpdate != null) {
+      builder.setDsData(updateDsData(existingDomain.getDsData(), secDnsUpdate));
+    }
+    return builder.build();
+  }
+
+  private DomainResource convertSunrushAddToAdd(
+      DomainResource newDomain, GracePeriod gracePeriod, HistoryEntry historyEntry) {
+    // Cancel the billing event for the sunrush add and replace it with a new billing event.
+    BillingEvent.Cancellation billingEventCancellation =
+        BillingEvent.Cancellation.forGracePeriod(gracePeriod, historyEntry, targetId);
+    BillingEvent.OneTime billingEvent =
+        createBillingEventForSunrushConversion(newDomain, historyEntry, gracePeriod);
+    ofy().save().entities(billingEvent, billingEventCancellation);
+    // Modify the grace periods on the domain.
+    return newDomain.asBuilder()
+        .removeGracePeriod(gracePeriod)
+        .addGracePeriod(GracePeriod.forBillingEvent(GracePeriodStatus.ADD, billingEvent))
+        .build();
+  }
+
+  private BillingEvent.OneTime createBillingEventForSunrushConversion(
+      DomainResource existingDomain, HistoryEntry historyEntry, GracePeriod sunrushAddGracePeriod) {
+    // Compute the expiration time of the add grace period. We will not allow it to be after the
+    // sunrush add grace period expiration time (i.e. you can't get extra add grace period by
+    // setting a nameserver).
+    DateTime addGracePeriodExpirationTime = earliestOf(
+        now.plus(Registry.get(existingDomain.getTld()).getAddGracePeriodLength()),
+        sunrushAddGracePeriod.getExpirationTime());
+    // Create a new billing event for the add grace period. Note that we do this even if it would
+    // occur at the same time as the sunrush add grace period, as the event time will differ
+    // between them.
+    BillingEvent.OneTime originalAddEvent =
+        ofy().load().key(sunrushAddGracePeriod.getOneTimeBillingEvent()).now();
+    return new BillingEvent.OneTime.Builder()
+        .setReason(Reason.CREATE)
+        .setTargetId(targetId)
+        .setFlags(originalAddEvent.getFlags())
+        .setClientId(sunrushAddGracePeriod.getClientId())
+        .setCost(originalAddEvent.getCost())
+        .setPeriodYears(originalAddEvent.getPeriodYears())
+        .setEventTime(now)
+        .setBillingTime(addGracePeriodExpirationTime)
+        .setParent(historyEntry)
+        .build();
+  }
+
+  private void validateNewState(DomainResource newDomain) throws EppException {
+    validateNoDuplicateContacts(newDomain.getContacts());
+    validateRequiredContactsPresent(newDomain.getRegistrant(), newDomain.getContacts());
+    validateDsData(newDomain.getDsData());
+    validateNameserversCountForTld(newDomain.getTld(), newDomain.getNameservers().size());
+  }
+
+  /** Some status updates cost money. Bill only once no matter how many of them are changed. */
+  private Optional<BillingEvent.OneTime> createBillingEventForStatusUpdates(
+      DomainResource existingDomain, DomainResource newDomain, HistoryEntry historyEntry) {
+    MetadataExtension metadataExtension = eppInput.getSingleExtension(MetadataExtension.class);
     if (metadataExtension != null && metadataExtension.getRequestedByRegistrar()) {
-      Set<StatusValue> statusDifferences =
+      for (StatusValue statusValue
-          symmetricDifference(existingResource.getStatusValues(), newResource.getStatusValues());
+          : symmetricDifference(existingDomain.getStatusValues(), newDomain.getStatusValues())) {
-      if (Iterables.any(statusDifferences, new Predicate<StatusValue>() {
+        if (statusValue.isChargedStatus()) {
-          @Override
+          // Only charge once.
-          public boolean apply(StatusValue statusValue) {
+          return Optional.of(new BillingEvent.OneTime.Builder()
-            return statusValue.isChargedStatus();
+              .setReason(Reason.SERVER_STATUS)
-          }})) {
+              .setTargetId(targetId)
-        BillingEvent.OneTime billingEvent = new BillingEvent.OneTime.Builder()
+              .setClientId(clientId)
-          .setReason(Reason.SERVER_STATUS)
+              .setCost(Registry.get(existingDomain.getTld()).getServerStatusChangeCost())
-          .setTargetId(targetId)
+              .setEventTime(now)
-          .setClientId(getClientId())
+              .setBillingTime(now)
-          .setCost(Registry.get(existingResource.getTld()).getServerStatusChangeCost())
+              .setParent(historyEntry)
-          .setEventTime(now)
+              .build());
-          .setBillingTime(now)
+        }
-          .setParent(historyEntry)
-          .build();
-        ofy().save().entity(billingEvent);
       }
     }
+    return Optional.absent();
   }
 
-  @Override
+  private void handleExtraFlowLogic(DomainResource existingDomain, HistoryEntry historyEntry)
-  protected void enqueueTasks() {
+      throws EppException {
-    DnsQueue.create().addDomainRefreshTask(existingResource.getFullyQualifiedDomainName());
+    Optional<RegistryExtraFlowLogic> extraFlowLogic =
-  }
+        RegistryExtraFlowLogicProxy.newInstanceForDomain(existingDomain);
-
+    if (extraFlowLogic.isPresent()) {
-  @Override
+      extraFlowLogic.get().performAdditionalDomainUpdateLogic(
-  protected final HistoryEntry.Type getHistoryEntryType() {
+          existingDomain, clientId, now, eppInput, historyEntry);
-    return HistoryEntry.Type.DOMAIN_UPDATE;
+      extraFlowLogic.get().commitAdditionalLogicChanges();
+    }
   }
 }

javatests/google/registry/flows/domain/DomainApplicationUpdateFlowTest.java

@@ -32,27 +32,27 @@ import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
 import google.registry.flows.EppException.UnimplementedExtensionException;
 import google.registry.flows.ResourceFlowTestCase;
+import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException;
 import google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException;
-import google.registry.flows.ResourceMutateFlow.ResourceDoesNotExistException;
-import google.registry.flows.ResourceUpdateFlow.AddRemoveSameValueEppException;
-import google.registry.flows.ResourceUpdateFlow.ResourceHasClientUpdateProhibitedException;
-import google.registry.flows.ResourceUpdateFlow.StatusNotClientSettableException;
-import google.registry.flows.SingleResourceFlow.ResourceStatusProhibitsOperationException;
-import google.registry.flows.domain.BaseDomainUpdateFlow.EmptySecDnsUpdateException;
-import google.registry.flows.domain.BaseDomainUpdateFlow.MaxSigLifeChangeNotSupportedException;
-import google.registry.flows.domain.BaseDomainUpdateFlow.SecDnsAllUsageException;
-import google.registry.flows.domain.BaseDomainUpdateFlow.UrgentAttributeNotSupportedException;
 import google.registry.flows.domain.DomainApplicationUpdateFlow.ApplicationStatusProhibitsUpdateException;
 import google.registry.flows.domain.DomainFlowUtils.DuplicateContactForRoleException;
+import google.registry.flows.domain.DomainFlowUtils.EmptySecDnsUpdateException;
 import google.registry.flows.domain.DomainFlowUtils.LinkedResourcesDoNotExistException;
+import google.registry.flows.domain.DomainFlowUtils.MaxSigLifeChangeNotSupportedException;
 import google.registry.flows.domain.DomainFlowUtils.MissingAdminContactException;
 import google.registry.flows.domain.DomainFlowUtils.MissingContactTypeException;
 import google.registry.flows.domain.DomainFlowUtils.MissingTechnicalContactException;
 import google.registry.flows.domain.DomainFlowUtils.NameserversNotAllowedException;
 import google.registry.flows.domain.DomainFlowUtils.NotAuthorizedForTldException;
 import google.registry.flows.domain.DomainFlowUtils.RegistrantNotAllowedException;
+import google.registry.flows.domain.DomainFlowUtils.SecDnsAllUsageException;
 import google.registry.flows.domain.DomainFlowUtils.TooManyDsRecordsException;
 import google.registry.flows.domain.DomainFlowUtils.TooManyNameserversException;
+import google.registry.flows.domain.DomainFlowUtils.UrgentAttributeNotSupportedException;
+import google.registry.flows.exceptions.AddRemoveSameValueEppException;
+import google.registry.flows.exceptions.ResourceHasClientUpdateProhibitedException;
+import google.registry.flows.exceptions.ResourceStatusProhibitsOperationException;
+import google.registry.flows.exceptions.StatusNotClientSettableException;
 import google.registry.model.contact.ContactResource;
 import google.registry.model.domain.DesignatedContact;
 import google.registry.model.domain.DesignatedContact.Type;
@@ -323,10 +323,10 @@ public class DomainApplicationUpdateFlowTest
 
   private void doSecDnsFailingTest(Class<? extends Exception> expectedException, String xmlFilename)
       throws Exception {
-    thrown.expect(expectedException);
     setEppInput(xmlFilename);
     persistReferencedEntities();
     persistNewApplication();
+    thrown.expect(expectedException);
     runFlow();
   }
 
@@ -357,7 +357,6 @@ public class DomainApplicationUpdateFlowTest
 
   @Test
   public void testFailure_secDnsTooManyDsRecords() throws Exception {
-    thrown.expect(TooManyDsRecordsException.class);
     ImmutableSet.Builder<DelegationSignerData> builder = new ImmutableSet.Builder<>();
     for (int i = 0; i < 8; ++i) {
       builder.add(DelegationSignerData.create(i, 2, 3, new byte[]{0, 1, 2}));
@@ -365,6 +364,7 @@ public class DomainApplicationUpdateFlowTest
 
     setEppInput("domain_update_sunrise_dsdata_add.xml");
     persistResource(newApplicationBuilder().setDsData(builder.build()).build());
+    thrown.expect(TooManyDsRecordsException.class);
     runFlow();
   }
 
@@ -383,64 +383,64 @@ public class DomainApplicationUpdateFlowTest
 
   @Test
   public void testFailure_tooManyNameservers() throws Exception {
-    thrown.expect(TooManyNameserversException.class);
-    setEppInput("domain_update_sunrise_add_nameserver.xml");
     persistReferencedEntities();
     persistApplication();
     // Modify application to have 13 nameservers. We will then remove one and add one in the test.
     modifyApplicationToHave13Nameservers();
+    setEppInput("domain_update_sunrise_add_nameserver.xml");
+    thrown.expect(TooManyNameserversException.class);
     runFlow();
   }
 
   @Test
   public void testFailure_wrongExtension() throws Exception {
-    thrown.expect(UnimplementedExtensionException.class);
     setEppInput("domain_update_sunrise_wrong_extension.xml");
+    thrown.expect(UnimplementedExtensionException.class);
     runFlow();
   }
 
   @Test
   public void testFailure_neverExisted() throws Exception {
+    persistReferencedEntities();
     thrown.expect(
         ResourceDoesNotExistException.class,
         String.format("(%s)", getUniqueIdFromCommand()));
-    persistReferencedEntities();
     runFlow();
   }
 
   @Test
   public void testFailure_existedButWasDeleted() throws Exception {
+    persistReferencedEntities();
+    persistResource(newApplicationBuilder().setDeletionTime(START_OF_TIME).build());
     thrown.expect(
         ResourceDoesNotExistException.class,
         String.format("(%s)", getUniqueIdFromCommand()));
-    persistReferencedEntities();
-    persistResource(newApplicationBuilder().setDeletionTime(START_OF_TIME).build());
     runFlow();
   }
 
   @Test
   public void testFailure_clientUpdateProhibited() throws Exception {
-    thrown.expect(ResourceHasClientUpdateProhibitedException.class);
     setEppInput("domain_update_sunrise_authinfo.xml");
     persistReferencedEntities();
     persistResource(newApplicationBuilder().setStatusValues(
         ImmutableSet.of(StatusValue.CLIENT_UPDATE_PROHIBITED)).build());
+    thrown.expect(ResourceHasClientUpdateProhibitedException.class);
     runFlow();
   }
 
   @Test
   public void testFailure_serverUpdateProhibited() throws Exception {
-    thrown.expect(ResourceStatusProhibitsOperationException.class);
     persistReferencedEntities();
     persistResource(newApplicationBuilder().setStatusValues(
         ImmutableSet.of(StatusValue.SERVER_UPDATE_PROHIBITED)).build());
+    thrown.expect(ResourceStatusProhibitsOperationException.class);
     runFlow();
   }
 
   private void doIllegalApplicationStatusTest(ApplicationStatus status) throws Exception {
-    thrown.expect(ApplicationStatusProhibitsUpdateException.class);
     persistReferencedEntities();
     persistResource(newApplicationBuilder().setApplicationStatus(status).build());
+    thrown.expect(ApplicationStatusProhibitsUpdateException.class);
     runFlow();
   }
 
@@ -461,31 +461,30 @@ public class DomainApplicationUpdateFlowTest
 
   @Test
   public void testFailure_missingHost() throws Exception {
-    thrown.expect(
-        LinkedResourcesDoNotExistException.class,
-        "(ns2.example.tld)");
     persistActiveHost("ns1.example.tld");
     persistActiveContact("sh8013");
     persistActiveContact("mak21");
     persistNewApplication();
+    thrown.expect(
+        LinkedResourcesDoNotExistException.class,
+        "(ns2.example.tld)");
     runFlow();
   }
 
   @Test
   public void testFailure_missingContact() throws Exception {
-    thrown.expect(
-        LinkedResourcesDoNotExistException.class,
-        "(sh8013)");
     persistActiveHost("ns1.example.tld");
     persistActiveHost("ns2.example.tld");
     persistActiveContact("mak21");
     persistNewApplication();
+    thrown.expect(
+        LinkedResourcesDoNotExistException.class,
+        "(sh8013)");
     runFlow();
   }
 
   @Test
   public void testFailure_addingDuplicateContact() throws Exception {
-    thrown.expect(DuplicateContactForRoleException.class);
     persistReferencedEntities();
     persistActiveContact("foo");
     persistNewApplication();
@@ -494,15 +493,16 @@ public class DomainApplicationUpdateFlowTest
     persistResource(reloadDomainApplication().asBuilder().setContacts(ImmutableSet.of(
         DesignatedContact.create(Type.TECH, Key.create(
             loadByForeignKey(ContactResource.class, "foo", clock.nowUtc()))))).build());
+    thrown.expect(DuplicateContactForRoleException.class);
     runFlow();
   }
 
   @Test
   public void testFailure_clientProhibitedStatusValue() throws Exception {
-    thrown.expect(StatusNotClientSettableException.class);
     setEppInput("domain_update_sunrise_prohibited_status.xml");
     persistReferencedEntities();
     persistNewApplication();
+    thrown.expect(StatusNotClientSettableException.class);
     runFlow();
   }
 
@@ -521,35 +521,34 @@ public class DomainApplicationUpdateFlowTest
 
   @Test
   public void testFailure_duplicateContactInCommand() throws Exception {
-    thrown.expect(DuplicateContactForRoleException.class);
     setEppInput("domain_update_sunrise_duplicate_contact.xml");
     persistReferencedEntities();
     persistNewApplication();
+    thrown.expect(DuplicateContactForRoleException.class);
     runFlow();
   }
 
   @Test
   public void testFailure_missingContactType() throws Exception {
-    // We need to test for missing type, but not for invalid - the schema enforces that for us.
-    thrown.expect(MissingContactTypeException.class);
     setEppInput("domain_update_sunrise_missing_contact_type.xml");
     persistReferencedEntities();
     persistNewApplication();
+    // We need to test for missing type, but not for invalid - the schema enforces that for us.
+    thrown.expect(MissingContactTypeException.class);
     runFlow();
   }
 
   @Test
   public void testFailure_unauthorizedClient() throws Exception {
-    thrown.expect(ResourceNotOwnedException.class);
     sessionMetadata.setClientId("NewRegistrar");
     persistReferencedEntities();
     persistApplication();
+    thrown.expect(ResourceNotOwnedException.class);
     runFlow();
   }
 
   @Test
   public void testFailure_notAuthorizedForTld() throws Exception {
-    thrown.expect(NotAuthorizedForTldException.class);
     persistResource(
         Registrar.loadByClientId("TheRegistrar")
             .asBuilder()
@@ -557,6 +556,7 @@ public class DomainApplicationUpdateFlowTest
             .build());
     persistReferencedEntities();
     persistApplication();
+    thrown.expect(NotAuthorizedForTldException.class);
     runFlow();
   }
 
@@ -572,19 +572,18 @@ public class DomainApplicationUpdateFlowTest
 
   @Test
   public void testFailure_sameNameserverAddedAndRemoved() throws Exception {
-    thrown.expect(AddRemoveSameValueEppException.class);
     setEppInput("domain_update_sunrise_add_remove_same_host.xml");
     persistReferencedEntities();
     persistResource(newApplicationBuilder()
         .setNameservers(ImmutableSet.of(Key.create(
             loadByForeignKey(HostResource.class, "ns1.example.tld", clock.nowUtc()))))
         .build());
+    thrown.expect(AddRemoveSameValueEppException.class);
     runFlow();
   }
 
   @Test
   public void testFailure_sameContactAddedAndRemoved() throws Exception {
-    thrown.expect(AddRemoveSameValueEppException.class);
     setEppInput("domain_update_sunrise_add_remove_same_contact.xml");
     persistReferencedEntities();
     persistResource(newApplicationBuilder()
@@ -593,12 +592,12 @@ public class DomainApplicationUpdateFlowTest
             Key.create(
                 loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc())))))
         .build());
+    thrown.expect(AddRemoveSameValueEppException.class);
     runFlow();
   }
 
   @Test
   public void testFailure_removeAdmin() throws Exception {
-    thrown.expect(MissingAdminContactException.class);
     setEppInput("domain_update_sunrise_remove_admin.xml");
     persistReferencedEntities();
     persistResource(newApplicationBuilder()
@@ -606,12 +605,12 @@ public class DomainApplicationUpdateFlowTest
             DesignatedContact.create(Type.ADMIN, Key.create(sh8013Contact)),
             DesignatedContact.create(Type.TECH, Key.create(sh8013Contact))))
         .build());
+    thrown.expect(MissingAdminContactException.class);
     runFlow();
   }
 
   @Test
   public void testFailure_removeTech() throws Exception {
-    thrown.expect(MissingTechnicalContactException.class);
     setEppInput("domain_update_sunrise_remove_tech.xml");
     persistReferencedEntities();
     persistResource(newApplicationBuilder()
@@ -619,6 +618,7 @@ public class DomainApplicationUpdateFlowTest
             DesignatedContact.create(Type.ADMIN, Key.create(sh8013Contact)),
             DesignatedContact.create(Type.TECH, Key.create(sh8013Contact))))
         .build());
+    thrown.expect(MissingTechnicalContactException.class);
     runFlow();
   }
 

javatests/google/registry/flows/domain/DomainUpdateFlowTest.java

@@ -43,23 +43,16 @@ import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
 import google.registry.flows.EppException.UnimplementedExtensionException;
 import google.registry.flows.EppRequestSource;
-import google.registry.flows.ResourceCreateOrMutateFlow.OnlyToolCanPassMetadataException;
 import google.registry.flows.ResourceFlowTestCase;
+import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException;
 import google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException;
-import google.registry.flows.ResourceMutateFlow.ResourceDoesNotExistException;
-import google.registry.flows.ResourceUpdateFlow.AddRemoveSameValueEppException;
-import google.registry.flows.ResourceUpdateFlow.ResourceHasClientUpdateProhibitedException;
-import google.registry.flows.ResourceUpdateFlow.StatusNotClientSettableException;
-import google.registry.flows.SingleResourceFlow.ResourceStatusProhibitsOperationException;
-import google.registry.flows.domain.BaseDomainUpdateFlow.EmptySecDnsUpdateException;
-import google.registry.flows.domain.BaseDomainUpdateFlow.MaxSigLifeChangeNotSupportedException;
-import google.registry.flows.domain.BaseDomainUpdateFlow.SecDnsAllUsageException;
-import google.registry.flows.domain.BaseDomainUpdateFlow.UrgentAttributeNotSupportedException;
 import google.registry.flows.domain.DomainFlowUtils.DuplicateContactForRoleException;
+import google.registry.flows.domain.DomainFlowUtils.EmptySecDnsUpdateException;
 import google.registry.flows.domain.DomainFlowUtils.FeesMismatchException;
 import google.registry.flows.domain.DomainFlowUtils.FeesRequiredForNonFreeUpdateException;
 import google.registry.flows.domain.DomainFlowUtils.LinkedResourceInPendingDeleteProhibitsOperationException;
 import google.registry.flows.domain.DomainFlowUtils.LinkedResourcesDoNotExistException;
+import google.registry.flows.domain.DomainFlowUtils.MaxSigLifeChangeNotSupportedException;
 import google.registry.flows.domain.DomainFlowUtils.MissingAdminContactException;
 import google.registry.flows.domain.DomainFlowUtils.MissingContactTypeException;
 import google.registry.flows.domain.DomainFlowUtils.MissingTechnicalContactException;
@@ -67,8 +60,15 @@ import google.registry.flows.domain.DomainFlowUtils.NameserversNotAllowedExcepti
 import google.registry.flows.domain.DomainFlowUtils.NameserversNotSpecifiedException;
 import google.registry.flows.domain.DomainFlowUtils.NotAuthorizedForTldException;
 import google.registry.flows.domain.DomainFlowUtils.RegistrantNotAllowedException;
+import google.registry.flows.domain.DomainFlowUtils.SecDnsAllUsageException;
 import google.registry.flows.domain.DomainFlowUtils.TooManyDsRecordsException;
 import google.registry.flows.domain.DomainFlowUtils.TooManyNameserversException;
+import google.registry.flows.domain.DomainFlowUtils.UrgentAttributeNotSupportedException;
+import google.registry.flows.exceptions.AddRemoveSameValueEppException;
+import google.registry.flows.exceptions.OnlyToolCanPassMetadataException;
+import google.registry.flows.exceptions.ResourceHasClientUpdateProhibitedException;
+import google.registry.flows.exceptions.ResourceStatusProhibitsOperationException;
+import google.registry.flows.exceptions.StatusNotClientSettableException;
 import google.registry.model.billing.BillingEvent;
 import google.registry.model.billing.BillingEvent.Reason;
 import google.registry.model.contact.ContactResource;
@@ -822,6 +822,8 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
   public void testFailure_wrongExtension() throws Exception {
     thrown.expect(UnimplementedExtensionException.class);
     setEppInput("domain_update_wrong_extension.xml");
+    persistReferencedEntities();
+    persistDomain();
     runFlow();
   }