From 07135f6190d548234364a3b4f093efc0b1b47403 Mon Sep 17 00:00:00 2001 From: mcilwain Date: Fri, 2 Sep 2016 08:37:45 -0700 Subject: [PATCH] Don't allow null in BillingEvent.setFlags() It's better that it always takes a non-null ImmutableSet, which may either be empty or contain elements. That way the ugliness of nullness is contained just to the entity class itself, and all other code that interacts with it can always be assured of having a real set to deal with. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=132066238 --- java/google/registry/flows/domain/DomainCreateFlow.java | 4 +++- java/google/registry/model/billing/BillingEvent.java | 6 +++++- .../google/registry/flows/domain/DomainCreateFlowTest.java | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/java/google/registry/flows/domain/DomainCreateFlow.java b/java/google/registry/flows/domain/DomainCreateFlow.java index 36917f60b..a8b0e0cab 100644 --- a/java/google/registry/flows/domain/DomainCreateFlow.java +++ b/java/google/registry/flows/domain/DomainCreateFlow.java @@ -157,7 +157,9 @@ public class DomainCreateFlow extends DomainCreateOrAllocateFlow { .setBillingTime(now.plus(isAnchorTenant() ? registry.getAnchorTenantAddGracePeriodLength() : registry.getAddGracePeriodLength())) - .setFlags(isAnchorTenant() ? ImmutableSet.of(BillingEvent.Flag.ANCHOR_TENANT) : null) + .setFlags(isAnchorTenant() + ? ImmutableSet.of(BillingEvent.Flag.ANCHOR_TENANT) + : ImmutableSet.of()) .setParent(historyEntry) .build(); ofy().save().entity(createEvent); diff --git a/java/google/registry/model/billing/BillingEvent.java b/java/google/registry/model/billing/BillingEvent.java index 3a1337172..bc6d430eb 100644 --- a/java/google/registry/model/billing/BillingEvent.java +++ b/java/google/registry/model/billing/BillingEvent.java @@ -19,9 +19,11 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.util.CollectionUtils.forceEmptyToNull; import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy; import static google.registry.util.CollectionUtils.union; import static google.registry.util.DateTimeUtils.END_OF_TIME; +import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import com.google.common.base.Optional; import com.google.common.collect.ImmutableMap; @@ -44,6 +46,7 @@ import google.registry.model.reporting.HistoryEntry; import google.registry.model.transfer.TransferData.TransferServerApproveEntity; import java.util.Objects; import java.util.Set; +import javax.annotation.Nullable; import org.joda.money.Money; import org.joda.time.DateTime; @@ -98,6 +101,7 @@ public abstract class BillingEvent extends ImmutableObject /** The fully qualified domain name of the domain that the bill is for. */ String targetId; + @Nullable Set flags; public String getClientId() { @@ -168,7 +172,7 @@ public abstract class BillingEvent extends ImmutableObject } public B setFlags(ImmutableSet flags) { - getInstance().flags = flags; + getInstance().flags = forceEmptyToNull(checkArgumentNotNull(flags, "flags")); return thisCastToDerived(); } diff --git a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java index 14580ad1b..202ce5b80 100644 --- a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java @@ -178,7 +178,7 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase billingFlags = isAnchorTenant ? ImmutableSet.of(BillingEvent.Flag.ANCHOR_TENANT) - : null; + : ImmutableSet.of(); HistoryEntry historyEntry = getHistoryEntries(domain).get(0); assertAboutDomains().that(domain) .hasRegistrationExpirationTime(