From 5368489987218865e3b3eaff3f7200dde76e9e22 Mon Sep 17 00:00:00 2001 From: mcilwain Date: Wed, 16 Nov 2016 09:25:44 -0800 Subject: [PATCH] Enforce nullness consistency on EppResponse.set...() methods The callsites were inconsistent between whether they were passing empty list or null, and many of the ones that were passing null were not correctly annotated with @Nullable. I'm now going with empty list throughout except for the final step where the actual field that will be transformed into XML is set, where it is coerced to null to avoid an empty element in the XML output. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=139340837 --- .../registry/flows/domain/DomainAllocateFlow.java | 2 +- .../registry/flows/domain/DomainCheckFlow.java | 2 +- .../registry/flows/domain/DomainCreateFlow.java | 2 +- .../registry/flows/domain/DomainDeleteFlow.java | 4 ++-- .../registry/flows/domain/DomainInfoFlow.java | 3 +-- .../registry/flows/domain/DomainRenewFlow.java | 11 ++++++----- .../flows/domain/DomainRestoreRequestFlow.java | 5 +++-- .../flows/domain/DomainTransferRequestFlow.java | 2 +- .../registry/flows/poll/PollRequestFlow.java | 5 ++--- .../registry/model/eppoutput/EppResponse.java | 15 +++++++++------ 10 files changed, 27 insertions(+), 24 deletions(-) diff --git a/java/google/registry/flows/domain/DomainAllocateFlow.java b/java/google/registry/flows/domain/DomainAllocateFlow.java index 413feba5b..54532356e 100644 --- a/java/google/registry/flows/domain/DomainAllocateFlow.java +++ b/java/google/registry/flows/domain/DomainAllocateFlow.java @@ -394,7 +394,7 @@ public class DomainAllocateFlow implements TransactionalFlow { FeeCreateCommandExtension feeCreate = eppInput.getSingleExtension(FeeCreateCommandExtension.class); return (feeCreate == null) - ? null + ? ImmutableList.of() : ImmutableList.of(createFeeCreateResponse(feeCreate, commandOperations)); } diff --git a/java/google/registry/flows/domain/DomainCheckFlow.java b/java/google/registry/flows/domain/DomainCheckFlow.java index bcfbb83f2..2089805bc 100644 --- a/java/google/registry/flows/domain/DomainCheckFlow.java +++ b/java/google/registry/flows/domain/DomainCheckFlow.java @@ -177,7 +177,7 @@ public final class DomainCheckFlow implements Flow { FeeCheckCommandExtension feeCheck = eppInput.getSingleExtension(FeeCheckCommandExtension.class); if (feeCheck == null) { - return null; // No fee checks were requested. + return ImmutableList.of(); // No fee checks were requested. } ImmutableList.Builder responseItems = new ImmutableList.Builder<>(); diff --git a/java/google/registry/flows/domain/DomainCreateFlow.java b/java/google/registry/flows/domain/DomainCreateFlow.java index 520e38fc7..a2da29949 100644 --- a/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/java/google/registry/flows/domain/DomainCreateFlow.java @@ -439,7 +439,7 @@ public class DomainCreateFlow implements TransactionalFlow { private ImmutableList createResponseExtensions( FeeCreateCommandExtension feeCreate, EppCommandOperations commandOperations) { return (feeCreate == null) - ? null + ? ImmutableList.of() : ImmutableList.of(createFeeCreateResponse(feeCreate, commandOperations)); } diff --git a/java/google/registry/flows/domain/DomainDeleteFlow.java b/java/google/registry/flows/domain/DomainDeleteFlow.java index e1d08d1a8..055aa4cdb 100644 --- a/java/google/registry/flows/domain/DomainDeleteFlow.java +++ b/java/google/registry/flows/domain/DomainDeleteFlow.java @@ -221,7 +221,7 @@ public final class DomainDeleteFlow implements TransactionalFlow { DomainResource existingDomain, DateTime now) { FeeTransformResponseExtension.Builder feeResponseBuilder = getDeleteResponseBuilder(); if (feeResponseBuilder == null) { - return null; + return ImmutableList.of(); } ImmutableList.Builder creditsBuilder = new ImmutableList.Builder<>(); for (GracePeriod gracePeriod : existingDomain.getGracePeriods()) { @@ -234,7 +234,7 @@ public final class DomainDeleteFlow implements TransactionalFlow { } ImmutableList credits = creditsBuilder.build(); if (credits.isEmpty()) { - return null; + return ImmutableList.of(); } return ImmutableList.of(feeResponseBuilder.setCredits(credits).build()); } diff --git a/java/google/registry/flows/domain/DomainInfoFlow.java b/java/google/registry/flows/domain/DomainInfoFlow.java index 5d0b1a353..447fb9eab 100644 --- a/java/google/registry/flows/domain/DomainInfoFlow.java +++ b/java/google/registry/flows/domain/DomainInfoFlow.java @@ -19,7 +19,6 @@ import static google.registry.flows.ResourceFlowUtils.loadAndVerifyExistence; import static google.registry.flows.ResourceFlowUtils.verifyOptionalAuthInfo; import static google.registry.flows.domain.DomainFlowUtils.addSecDnsExtensionIfPresent; import static google.registry.flows.domain.DomainFlowUtils.handleFeeRequest; -import static google.registry.util.CollectionUtils.forceEmptyToNull; import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; @@ -149,6 +148,6 @@ public final class DomainInfoFlow implements Flow { extensions.add(FlagsInfoResponseExtension.create(ImmutableList.copyOf(flags))); } } - return forceEmptyToNull(extensions.build()); + return extensions.build(); } } diff --git a/java/google/registry/flows/domain/DomainRenewFlow.java b/java/google/registry/flows/domain/DomainRenewFlow.java index a9a5f8a5a..f6af5a3c9 100644 --- a/java/google/registry/flows/domain/DomainRenewFlow.java +++ b/java/google/registry/flows/domain/DomainRenewFlow.java @@ -216,11 +216,12 @@ public final class DomainRenewFlow implements TransactionalFlow { private ImmutableList createResponseExtensions( Money renewCost, FeeRenewCommandExtension feeRenew) { - return (feeRenew == null) ? null : ImmutableList.of(feeRenew - .createResponseBuilder() - .setCurrency(renewCost.getCurrencyUnit()) - .setFees(ImmutableList.of(Fee.create(renewCost.getAmount(), FeeType.RENEW))) - .build()); + return (feeRenew == null) + ? ImmutableList.of() + : ImmutableList.of(feeRenew.createResponseBuilder() + .setCurrency(renewCost.getCurrencyUnit()) + .setFees(ImmutableList.of(Fee.create(renewCost.getAmount(), FeeType.RENEW))) + .build()); } /** The domain has a pending transfer on it and so can't be explicitly renewed. */ diff --git a/java/google/registry/flows/domain/DomainRestoreRequestFlow.java b/java/google/registry/flows/domain/DomainRestoreRequestFlow.java index 94a1958b3..8beaa5ef8 100644 --- a/java/google/registry/flows/domain/DomainRestoreRequestFlow.java +++ b/java/google/registry/flows/domain/DomainRestoreRequestFlow.java @@ -260,8 +260,9 @@ public final class DomainRestoreRequestFlow implements TransactionalFlow { private static ImmutableList createResponseExtensions( Money restoreCost, Money renewCost, FeeUpdateCommandExtension feeUpdate) { - return (feeUpdate == null) ? null : ImmutableList.of( - feeUpdate.createResponseBuilder() + return (feeUpdate == null) + ? ImmutableList.of() + : ImmutableList.of(feeUpdate.createResponseBuilder() .setCurrency(restoreCost.getCurrencyUnit()) .setFees(ImmutableList.of( Fee.create(restoreCost.getAmount(), FeeType.RESTORE), diff --git a/java/google/registry/flows/domain/DomainTransferRequestFlow.java b/java/google/registry/flows/domain/DomainTransferRequestFlow.java index 6c0c2d9cd..053783365 100644 --- a/java/google/registry/flows/domain/DomainTransferRequestFlow.java +++ b/java/google/registry/flows/domain/DomainTransferRequestFlow.java @@ -408,7 +408,7 @@ public final class DomainTransferRequestFlow implements TransactionalFlow { private ImmutableList createResponseExtensions(Money renewCost, FeeTransferCommandExtension feeTransfer) { return feeTransfer == null - ? null + ? ImmutableList.of() : ImmutableList.of(feeTransfer.createResponseBuilder() .setCurrency(renewCost.getCurrencyUnit()) .setFees(ImmutableList.of(Fee.create(renewCost.getAmount(), FeeType.RENEW))) diff --git a/java/google/registry/flows/poll/PollRequestFlow.java b/java/google/registry/flows/poll/PollRequestFlow.java index 822b20e38..af4bca6a5 100644 --- a/java/google/registry/flows/poll/PollRequestFlow.java +++ b/java/google/registry/flows/poll/PollRequestFlow.java @@ -18,7 +18,6 @@ import static google.registry.flows.FlowUtils.validateClientIsLoggedIn; import static google.registry.flows.poll.PollFlowUtils.getPollMessagesQuery; import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_ACK_MESSAGE; import static google.registry.model.eppoutput.Result.Code.SUCCESS_WITH_NO_MESSAGES; -import static google.registry.util.CollectionUtils.forceEmptyToNull; import com.googlecode.objectify.Key; import google.registry.flows.EppException; @@ -76,8 +75,8 @@ public class PollRequestFlow implements Flow { .setQueueLength(getPollMessagesQuery(clientId, now).count()) .setMessageId(PollMessage.EXTERNAL_KEY_CONVERTER.convert(Key.create(pollMessage))) .build()) - .setMultipleResData(forceEmptyToNull(pollMessage.getResponseData())) - .setExtensions(forceEmptyToNull(pollMessage.getResponseExtensions())) + .setMultipleResData(pollMessage.getResponseData()) + .setExtensions(pollMessage.getResponseExtensions()) .build(); } diff --git a/java/google/registry/model/eppoutput/EppResponse.java b/java/google/registry/model/eppoutput/EppResponse.java index 49352ba89..736e47396 100644 --- a/java/google/registry/model/eppoutput/EppResponse.java +++ b/java/google/registry/model/eppoutput/EppResponse.java @@ -14,6 +14,9 @@ package google.registry.model.eppoutput; +import static google.registry.util.CollectionUtils.forceEmptyToNull; +import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy; + import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableList; import google.registry.model.Buildable; @@ -150,11 +153,11 @@ public class EppResponse extends ImmutableObject implements ResponseOrGreeting { ImmutableList extensions; public ImmutableList getResponseData() { - return resData; + return nullToEmptyImmutableCopy(resData); } public ImmutableList getExtensions() { - return extensions; + return nullToEmptyImmutableCopy(extensions); } @Nullable @@ -216,8 +219,8 @@ public class EppResponse extends ImmutableObject implements ResponseOrGreeting { return setMultipleResData(ImmutableList.of(onlyResData)); } - public Builder setMultipleResData(@Nullable ImmutableList resData) { - getInstance().resData = resData; + public Builder setMultipleResData(ImmutableList resData) { + getInstance().resData = forceEmptyToNull(resData); return this; } @@ -225,8 +228,8 @@ public class EppResponse extends ImmutableObject implements ResponseOrGreeting { return setExtensions(ImmutableList.of(onlyExtension)); } - public Builder setExtensions(@Nullable ImmutableList extensions) { - getInstance().extensions = extensions; + public Builder setExtensions(ImmutableList extensions) { + getInstance().extensions = forceEmptyToNull(extensions); return this; } }