From 561be028c4483f19041bc3892a504a0a144c6a76 Mon Sep 17 00:00:00 2001 From: Lai Jiang Date: Thu, 15 Sep 2022 14:07:38 -0400 Subject: [PATCH] Remove generics from TransferData (#1787) `TransferData` is currently a generic class with a complicated type parameter that designate the `Builder` class of its concrete subclass, on order to facilitate returning the said `Builder` from an instance loosely typed to the superclass (`TransferData`) itself. While this works, in most all places that a `TransferData` is used, the raw, un-generic type is declared, resulting a lot of warnings, not to mention the fact that type safety not actually checked when raw type is used. In this PR, we make it so that the concrete `Builder` is returned through a protected abstract method that is implemented by the subclasses. The type information therefore no longer needs to be embedded in the superclass type signature, and reflection is not necessary to create the `Builder` either. Overall, it makes `TransferData` a much cleaner class without the messiness of generics. --- .../flows/domain/DomainTransferUtils.java | 2 +- .../model/transfer/ContactTransferData.java | 7 ++++++- .../model/transfer/DomainTransferData.java | 9 +++++++-- .../registry/model/transfer/TransferData.java | 16 +++++++--------- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/google/registry/flows/domain/DomainTransferUtils.java b/core/src/main/java/google/registry/flows/domain/DomainTransferUtils.java index 57ffd7f83..f04d2d34c 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainTransferUtils.java +++ b/core/src/main/java/google/registry/flows/domain/DomainTransferUtils.java @@ -274,7 +274,7 @@ public final class DomainTransferUtils { * renewal, we must issue a cancellation for the autorenew, so that the losing registrar will not * be charged (essentially, the gaining registrar takes on the cost of the year of registration * that the autorenew just added). But, if the superuser extension is used to request a transfer - * without an additional year then the gaining registrar is not charged for the one year renewal + * without an additional year then the gaining registrar is not charged for the one-year renewal * and the losing registrar still needs to be charged for the auto-renew. * *

For details on the policy justification, see b/19430703#comment17 and { +public class ContactTransferData extends TransferData { public static final ContactTransferData EMPTY = new ContactTransferData(); @Override @@ -26,6 +26,11 @@ public class ContactTransferData extends TransferData { +public class DomainTransferData extends TransferData { public static final DomainTransferData EMPTY = new DomainTransferData(); /** @@ -107,7 +107,7 @@ public class DomainTransferData extends TransferData @Override public Builder copyConstantFieldsToBuilder() { - return super.copyConstantFieldsToBuilder().setTransferPeriod(transferPeriod); + return ((Builder) super.copyConstantFieldsToBuilder()).setTransferPeriod(transferPeriod); } public Period getTransferPeriod() { @@ -157,6 +157,11 @@ public class DomainTransferData extends TransferData return EMPTY.equals(this); } + @Override + protected Builder createEmptyBuilder() { + return new Builder(); + } + @Override public Builder asBuilder() { return new Builder(clone(this)); diff --git a/core/src/main/java/google/registry/model/transfer/TransferData.java b/core/src/main/java/google/registry/model/transfer/TransferData.java index 05efa8cd3..55c56103b 100644 --- a/core/src/main/java/google/registry/model/transfer/TransferData.java +++ b/core/src/main/java/google/registry/model/transfer/TransferData.java @@ -27,7 +27,6 @@ import google.registry.model.eppcommon.Trid; import google.registry.model.poll.PollMessage; import google.registry.persistence.VKey; import google.registry.util.NullIgnoringCollectionBuilder; -import google.registry.util.TypeUtils.TypeInstantiator; import java.util.Set; import javax.annotation.Nullable; import javax.persistence.AttributeOverride; @@ -41,10 +40,7 @@ import javax.persistence.MappedSuperclass; * are implicitly transferred with their superordinate domain. */ @MappedSuperclass -public abstract class TransferData< - B extends - TransferData.Builder, ? extends TransferData.Builder>> - extends BaseTransferObject implements Buildable { +public abstract class TransferData extends BaseTransferObject implements Buildable { /** The transaction id of the most recent transfer request (or null if there never was one). */ @Embedded @@ -102,6 +98,8 @@ public abstract class TransferData< @Override public abstract Builder asBuilder(); + protected abstract Builder createEmptyBuilder(); + /** * Returns a fresh Builder populated only with the constant fields of this TransferData, i.e. * those that are fixed and unchanging throughout the transfer process. @@ -116,8 +114,8 @@ public abstract class TransferData< *

  • transferPeriod * */ - public B copyConstantFieldsToBuilder() { - B newBuilder = new TypeInstantiator(getClass()) {}.instantiate(); + public Builder copyConstantFieldsToBuilder() { + Builder newBuilder = createEmptyBuilder(); newBuilder .setTransferRequestTrid(transferRequestTrid) .setTransferRequestTime(transferRequestTime) @@ -129,7 +127,7 @@ public abstract class TransferData< /** Maps serverApproveEntities set to the individual fields. */ static void mapServerApproveEntitiesToFields( Set> serverApproveEntities, - TransferData transferData) { + TransferData transferData) { if (isNullOrEmpty(serverApproveEntities)) { transferData.pollMessageId1 = null; transferData.pollMessageId2 = null; @@ -161,7 +159,7 @@ public abstract class TransferData< } /** Builder for {@link TransferData} because it is immutable. */ - public abstract static class Builder, B extends Builder> + public abstract static class Builder> extends BaseTransferObject.Builder { /** Create a {@link Builder} wrapping a new instance. */