diff --git a/core/src/main/java/google/registry/model/EppResource.java b/core/src/main/java/google/registry/model/EppResource.java index 753d319d1..fda0c6879 100644 --- a/core/src/main/java/google/registry/model/EppResource.java +++ b/core/src/main/java/google/registry/model/EppResource.java @@ -155,9 +155,16 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable { return repoId; } - /** This method exists solely to satisfy Hibernate. Use {@link Builder} instead. */ - @SuppressWarnings("UnusedMethod") - private void setRepoId(String repoId) { + /** + * Sets the repository ID. + * + *

This should only be used for restoring the repo id of an object being loaded in a PostLoad + * method (effectively, when it is still under construction by Hibernate). In all other cases, the + * object should be regarded as immutable and changes should go through a Builder. + * + *

In addition to this special case use, this method must exist to satisfy Hibernate. + */ + public void setRepoId(String repoId) { this.repoId = repoId; } diff --git a/core/src/main/java/google/registry/model/contact/ContactHistory.java b/core/src/main/java/google/registry/model/contact/ContactHistory.java index 033a51329..61bc07119 100644 --- a/core/src/main/java/google/registry/model/contact/ContactHistory.java +++ b/core/src/main/java/google/registry/model/contact/ContactHistory.java @@ -115,7 +115,10 @@ public class ContactHistory extends HistoryEntry implements SqlEntity { contactBase = null; } if (contactBase != null && contactBase.getRepoId() == null) { - contactBase = contactBase.asBuilder().setRepoId(parent.getName()).build(); + // contactBase hasn't been fully constructed yet, so it's ok to go in and mutate it. Though + // the use of the Builder is not necessarily problematic in this case, this is still safer as + // the Builder can do things like comparisons that compute the hash code. + contactBase.setRepoId(parent.getName()); } } diff --git a/core/src/main/java/google/registry/model/domain/DomainHistory.java b/core/src/main/java/google/registry/model/domain/DomainHistory.java index 0eb6af37f..d96388f10 100644 --- a/core/src/main/java/google/registry/model/domain/DomainHistory.java +++ b/core/src/main/java/google/registry/model/domain/DomainHistory.java @@ -258,7 +258,10 @@ public class DomainHistory extends HistoryEntry implements SqlEntity { domainContent = null; } else { if (domainContent.getRepoId() == null) { - domainContent = domainContent.asBuilder().setRepoId(parent.getName()).build(); + // domainContent still hasn't been fully constructed yet, so it's ok to go in and mutate + // it. In fact, we have to because going through the builder causes the hash codes of + // contained objects to be calculated prematurely. + domainContent.setRepoId(parent.getName()); } } } diff --git a/core/src/main/java/google/registry/model/host/HostHistory.java b/core/src/main/java/google/registry/model/host/HostHistory.java index 2526073b8..0930f03c2 100644 --- a/core/src/main/java/google/registry/model/host/HostHistory.java +++ b/core/src/main/java/google/registry/model/host/HostHistory.java @@ -116,7 +116,10 @@ public class HostHistory extends HistoryEntry implements SqlEntity { hostBase = null; } if (hostBase != null && hostBase.getRepoId() == null) { - hostBase = hostBase.asBuilder().setRepoId(parent.getName()).build(); + // hostBase hasn't been fully constructed yet, so it's ok to go in and mutate it. Though the + // use of the Builder is not necessarily problematic in this case, this is still safer as the + // Builder can do things like comparisons that compute the hash code. + hostBase.setRepoId(parent.getName()); } } diff --git a/core/src/main/java/google/registry/model/transfer/DomainTransferData.java b/core/src/main/java/google/registry/model/transfer/DomainTransferData.java index 397746243..6696bddd8 100644 --- a/core/src/main/java/google/registry/model/transfer/DomainTransferData.java +++ b/core/src/main/java/google/registry/model/transfer/DomainTransferData.java @@ -164,7 +164,6 @@ public class DomainTransferData extends TransferData serverApproveEntities = null; postLoad(); } - hashCode = null; // reset the hash code since we may have changed the entities } /**