From cc2ea6851c37f5b582de32b023c92c1ea6aae939 Mon Sep 17 00:00:00 2001 From: Shicong Huang Date: Thu, 1 Oct 2020 11:01:57 -0400 Subject: [PATCH] Use composite primary key for HostHistory and ContactHistory (#809) * Use composite primary key for HostHistory and ContactHistory * Update flyway file version * Make getters private * Add javadoc * Rebase on HEAD --- .../model/contact/ContactHistory.java | 87 ++++++++++++++++--- .../model/contact/ContactResource.java | 1 - .../registry/model/domain/DomainHistory.java | 44 ++++++++-- .../registry/model/host/HostHistory.java | 85 +++++++++++++++--- .../model/reporting/HistoryEntry.java | 13 +-- .../model/history/ContactHistoryTest.java | 25 +++--- .../model/history/DomainHistoryTest.java | 5 +- .../model/history/HostHistoryTest.java | 23 ++--- .../history/LegacyHistoryObjectTest.java | 10 +-- .../google/registry/model/schema.txt | 4 +- db/src/main/resources/sql/flyway.txt | 1 + ...key_for_contact_and_host_history_table.sql | 23 +++++ .../sql/schema/db-schema.sql.generated | 12 +-- .../resources/sql/schema/nomulus.golden.sql | 4 +- 14 files changed, 248 insertions(+), 89 deletions(-) create mode 100644 db/src/main/resources/sql/flyway/V59__use_composite_primary_key_for_contact_and_host_history_table.sql 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 7a103e7fe..829952d32 100644 --- a/core/src/main/java/google/registry/model/contact/ContactHistory.java +++ b/core/src/main/java/google/registry/model/contact/ContactHistory.java @@ -17,8 +17,11 @@ package google.registry.model.contact; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.EntitySubclass; import google.registry.model.EppResource; +import google.registry.model.ImmutableObject; +import google.registry.model.contact.ContactHistory.ContactHistoryId; import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.VKey; +import java.io.Serializable; import java.util.Optional; import javax.annotation.Nullable; import javax.persistence.Access; @@ -28,6 +31,7 @@ import javax.persistence.Entity; import javax.persistence.GeneratedValue; import javax.persistence.GenerationType; import javax.persistence.Id; +import javax.persistence.IdClass; import javax.persistence.PostLoad; /** @@ -47,13 +51,13 @@ import javax.persistence.PostLoad; }) @EntitySubclass @Access(AccessType.FIELD) +@IdClass(ContactHistoryId.class) public class ContactHistory extends HistoryEntry { // Store ContactBase instead of ContactResource so we don't pick up its @Id @Nullable ContactBase contactBase; - @Column(nullable = false) - VKey contactRepoId; + @Id String contactRepoId; @Id @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "TempHistorySequenceGenerator") @@ -76,7 +80,14 @@ public class ContactHistory extends HistoryEntry { /** The key to the {@link ContactResource} this is based off of. */ public VKey getContactRepoId() { - return contactRepoId; + return VKey.create( + ContactResource.class, contactRepoId, Key.create(ContactResource.class, contactRepoId)); + } + + /** Creates a {@link VKey} instance for this entity. */ + public VKey createVKey() { + return VKey.create( + ContactHistory.class, new ContactHistoryId(contactRepoId, getId()), Key.create(this)); } @PostLoad @@ -87,10 +98,65 @@ public class ContactHistory extends HistoryEntry { contactBase = null; } // Fill in the full, symmetric, parent repo ID key - Key parentKey = - Key.create(ContactResource.class, (String) contactRepoId.getSqlKey()); - parent = parentKey; - contactRepoId = VKey.create(ContactResource.class, contactRepoId.getSqlKey(), parentKey); + parent = Key.create(ContactResource.class, contactRepoId); + } + + /** Class to represent the composite primary key of {@link ContactHistory} entity. */ + static class ContactHistoryId extends ImmutableObject implements Serializable { + + private String contactRepoId; + + private Long id; + + /** Hibernate requires this default constructor. */ + private ContactHistoryId() {} + + ContactHistoryId(String contactRepoId, long id) { + this.contactRepoId = contactRepoId; + this.id = id; + } + + /** + * Returns the contact repository id. + * + *

This method is private because it is only used by Hibernate. + */ + @SuppressWarnings("unused") + private String getContactRepoId() { + return contactRepoId; + } + + /** + * Returns the history revision id. + * + *

This method is private because it is only used by Hibernate. + */ + @SuppressWarnings("unused") + private long getId() { + return id; + } + + /** + * Sets the contact repository id. + * + *

This method is private because it is only used by Hibernate and should not be used + * externally to keep immutability. + */ + @SuppressWarnings("unused") + private void setContactRepoId(String contactRepoId) { + this.contactRepoId = contactRepoId; + } + + /** + * Sets the history revision id. + * + *

This method is private because it is only used by Hibernate and should not be used + * externally to keep immutability. + */ + @SuppressWarnings("unused") + private void setId(long id) { + this.id = id; + } } @Override @@ -111,9 +177,9 @@ public class ContactHistory extends HistoryEntry { return this; } - public Builder setContactRepoId(VKey contactRepoId) { + public Builder setContactRepoId(String contactRepoId) { getInstance().contactRepoId = contactRepoId; - contactRepoId.maybeGetOfyKey().ifPresent(parent -> getInstance().parent = parent); + getInstance().parent = Key.create(ContactResource.class, contactRepoId); return this; } @@ -121,8 +187,7 @@ public class ContactHistory extends HistoryEntry { @Override public Builder setParent(Key parent) { super.setParent(parent); - getInstance().contactRepoId = - VKey.create(ContactResource.class, parent.getName(), (Key) parent); + getInstance().contactRepoId = parent.getName(); return this; } } diff --git a/core/src/main/java/google/registry/model/contact/ContactResource.java b/core/src/main/java/google/registry/model/contact/ContactResource.java index 6cc66b8f8..c41feea20 100644 --- a/core/src/main/java/google/registry/model/contact/ContactResource.java +++ b/core/src/main/java/google/registry/model/contact/ContactResource.java @@ -51,7 +51,6 @@ public class ContactResource extends ContactBase @Override public VKey createVKey() { - // TODO(mmuller): create symmetric keys if we can ever reload both sides. return VKey.create(ContactResource.class, getRepoId(), Key.create(this)); } 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 ac926f408..a251cfe20 100644 --- a/core/src/main/java/google/registry/model/domain/DomainHistory.java +++ b/core/src/main/java/google/registry/model/domain/DomainHistory.java @@ -156,8 +156,10 @@ public class DomainHistory extends HistoryEntry { return VKey.create(DomainBase.class, domainRepoId, Key.create(DomainBase.class, domainRepoId)); } + /** Creates a {@link VKey} instance for this entity. */ public VKey createVKey() { - return VKey.createSql(DomainHistory.class, new DomainHistoryId(domainRepoId, getId())); + return VKey.create( + DomainHistory.class, new DomainHistoryId(domainRepoId, getId()), Key.create(this)); } @PostLoad @@ -188,19 +190,45 @@ public class DomainHistory extends HistoryEntry { this.id = id; } - String getDomainRepoId() { + /** + * Returns the domain repository id. + * + *

This method is private because it is only used by Hibernate. + */ + @SuppressWarnings("unused") + private String getDomainRepoId() { return domainRepoId; } - void setDomainRepoId(String domainRepoId) { - this.domainRepoId = domainRepoId; - } - - long getId() { + /** + * Returns the history revision id. + * + *

This method is private because it is only used by Hibernate. + */ + @SuppressWarnings("unused") + private long getId() { return id; } - void setId(long id) { + /** + * Sets the domain repository id. + * + *

This method is private because it is only used by Hibernate and should not be used + * externally to keep immutability. + */ + @SuppressWarnings("unused") + private void setDomainRepoId(String domainRepoId) { + this.domainRepoId = domainRepoId; + } + + /** + * Sets the history revision id. + * + *

This method is private because it is only used by Hibernate and should not be used + * externally to keep immutability. + */ + @SuppressWarnings("unused") + private void setId(long id) { this.id = id; } } 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 65e32d730..0490411bf 100644 --- a/core/src/main/java/google/registry/model/host/HostHistory.java +++ b/core/src/main/java/google/registry/model/host/HostHistory.java @@ -14,11 +14,15 @@ package google.registry.model.host; + import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.EntitySubclass; import google.registry.model.EppResource; +import google.registry.model.ImmutableObject; +import google.registry.model.host.HostHistory.HostHistoryId; import google.registry.model.reporting.HistoryEntry; import google.registry.persistence.VKey; +import java.io.Serializable; import java.util.Optional; import javax.annotation.Nullable; import javax.persistence.Access; @@ -28,6 +32,7 @@ import javax.persistence.Entity; import javax.persistence.GeneratedValue; import javax.persistence.GenerationType; import javax.persistence.Id; +import javax.persistence.IdClass; import javax.persistence.PostLoad; /** @@ -48,13 +53,13 @@ import javax.persistence.PostLoad; }) @EntitySubclass @Access(AccessType.FIELD) +@IdClass(HostHistoryId.class) public class HostHistory extends HistoryEntry { // Store HostBase instead of HostResource so we don't pick up its @Id @Nullable HostBase hostBase; - @Column(nullable = false) - VKey hostRepoId; + @Id String hostRepoId; @Id @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "TempHistorySequenceGenerator") @@ -77,7 +82,12 @@ public class HostHistory extends HistoryEntry { /** The key to the {@link google.registry.model.host.HostResource} this is based off of. */ public VKey getHostRepoId() { - return hostRepoId; + return VKey.create(HostResource.class, hostRepoId, Key.create(HostResource.class, hostRepoId)); + } + + /** Creates a {@link VKey} instance for this entity. */ + public VKey createVKey() { + return VKey.create(HostHistory.class, new HostHistoryId(hostRepoId, getId()), Key.create(this)); } @PostLoad @@ -88,9 +98,65 @@ public class HostHistory extends HistoryEntry { hostBase = null; } // Fill in the full, symmetric, parent repo ID key - Key parentKey = Key.create(HostResource.class, (String) hostRepoId.getSqlKey()); - parent = parentKey; - hostRepoId = VKey.create(HostResource.class, hostRepoId.getSqlKey(), parentKey); + parent = Key.create(HostResource.class, hostRepoId); + } + + /** Class to represent the composite primary key of {@link HostHistory} entity. */ + static class HostHistoryId extends ImmutableObject implements Serializable { + + private String hostRepoId; + + private Long id; + + /** Hibernate requires this default constructor. */ + private HostHistoryId() {} + + HostHistoryId(String hostRepoId, long id) { + this.hostRepoId = hostRepoId; + this.id = id; + } + + /** + * Returns the host repository id. + * + *

This method is private because it is only used by Hibernate. + */ + @SuppressWarnings("unused") + private String getHostRepoId() { + return hostRepoId; + } + + /** + * Returns the history revision id. + * + *

This method is private because it is only used by Hibernate. + */ + @SuppressWarnings("unused") + private long getId() { + return id; + } + + /** + * Sets the host repository id. + * + *

This method is private because it is only used by Hibernate and should not be used + * externally to keep immutability. + */ + @SuppressWarnings("unused") + private void setHostRepoId(String hostRepoId) { + this.hostRepoId = hostRepoId; + } + + /** + * Sets the history revision id. + * + *

This method is private because it is only used by Hibernate and should not be used + * externally to keep immutability. + */ + @SuppressWarnings("unused") + private void setId(long id) { + this.id = id; + } } @Override @@ -111,9 +177,9 @@ public class HostHistory extends HistoryEntry { return this; } - public Builder setHostRepoId(VKey hostRepoId) { + public Builder setHostRepoId(String hostRepoId) { getInstance().hostRepoId = hostRepoId; - hostRepoId.maybeGetOfyKey().ifPresent(parent -> getInstance().parent = parent); + getInstance().parent = Key.create(HostResource.class, hostRepoId); return this; } @@ -121,8 +187,7 @@ public class HostHistory extends HistoryEntry { @Override public Builder setParent(Key parent) { super.setParent(parent); - getInstance().hostRepoId = - VKey.create(HostResource.class, parent.getName(), (Key) parent); + getInstance().hostRepoId = parent.getName(); return this; } } diff --git a/core/src/main/java/google/registry/model/reporting/HistoryEntry.java b/core/src/main/java/google/registry/model/reporting/HistoryEntry.java index b5670f903..759cb653f 100644 --- a/core/src/main/java/google/registry/model/reporting/HistoryEntry.java +++ b/core/src/main/java/google/registry/model/reporting/HistoryEntry.java @@ -308,19 +308,10 @@ public class HistoryEntry extends ImmutableObject implements Buildable, Datastor new DomainHistory.Builder().copyFrom(this).setDomainRepoId(parent.getName()).build(); } else if (parentKind.equals(getKind(HostResource.class))) { resultEntity = - new HostHistory.Builder() - .copyFrom(this) - .setHostRepoId( - VKey.create(HostResource.class, parent.getName(), (Key) parent)) - .build(); + new HostHistory.Builder().copyFrom(this).setHostRepoId(parent.getName()).build(); } else if (parentKind.equals(getKind(ContactResource.class))) { resultEntity = - new ContactHistory.Builder() - .copyFrom(this) - .setContactRepoId( - VKey.create( - ContactResource.class, parent.getName(), (Key) parent)) - .build(); + new ContactHistory.Builder().copyFrom(this).setContactRepoId(parent.getName()).build(); } else { throw new IllegalStateException( String.format("Unknown kind of HistoryEntry parent %s", parentKind)); diff --git a/core/src/test/java/google/registry/model/history/ContactHistoryTest.java b/core/src/test/java/google/registry/model/history/ContactHistoryTest.java index 198c60d45..fdb73c29f 100644 --- a/core/src/test/java/google/registry/model/history/ContactHistoryTest.java +++ b/core/src/test/java/google/registry/model/history/ContactHistoryTest.java @@ -47,14 +47,13 @@ public class ContactHistoryTest extends EntityTestCase { jpaTm().transact(() -> jpaTm().insert(contact)); VKey contactVKey = contact.createVKey(); ContactResource contactFromDb = jpaTm().transact(() -> jpaTm().load(contactVKey)); - ContactHistory contactHistory = createContactHistory(contactFromDb, contactVKey); + ContactHistory contactHistory = createContactHistory(contactFromDb, contact.getRepoId()); contactHistory.id = null; jpaTm().transact(() -> jpaTm().insert(contactHistory)); jpaTm() .transact( () -> { - ContactHistory fromDatabase = - jpaTm().load(VKey.createSql(ContactHistory.class, contactHistory.getId())); + ContactHistory fromDatabase = jpaTm().load(contactHistory.createVKey()); assertContactHistoriesEqual(fromDatabase, contactHistory); assertThat(fromDatabase.getContactRepoId().getSqlKey()) .isEqualTo(contactHistory.getContactRepoId().getSqlKey()); @@ -70,15 +69,17 @@ public class ContactHistoryTest extends EntityTestCase { VKey contactVKey = contact.createVKey(); ContactResource contactFromDb = jpaTm().transact(() -> jpaTm().load(contactVKey)); ContactHistory contactHistory = - createContactHistory(contactFromDb, contactVKey).asBuilder().setContactBase(null).build(); + createContactHistory(contactFromDb, contact.getRepoId()) + .asBuilder() + .setContactBase(null) + .build(); contactHistory.id = null; jpaTm().transact(() -> jpaTm().insert(contactHistory)); jpaTm() .transact( () -> { - ContactHistory fromDatabase = - jpaTm().load(VKey.createSql(ContactHistory.class, contactHistory.getId())); + ContactHistory fromDatabase = jpaTm().load(contactHistory.createVKey()); assertContactHistoriesEqual(fromDatabase, contactHistory); assertThat(fromDatabase.getContactRepoId().getSqlKey()) .isEqualTo(contactHistory.getContactRepoId().getSqlKey()); @@ -94,14 +95,13 @@ public class ContactHistoryTest extends EntityTestCase { VKey contactVKey = contact.createVKey(); ContactResource contactFromDb = tm().transact(() -> tm().load(contactVKey)); fakeClock.advanceOneMilli(); - ContactHistory contactHistory = createContactHistory(contactFromDb, contactVKey); + ContactHistory contactHistory = createContactHistory(contactFromDb, contact.getRepoId()); tm().transact(() -> tm().insert(contactHistory)); // retrieving a HistoryEntry or a ContactHistory with the same key should return the same object // note: due to the @EntitySubclass annotation. all Keys for ContactHistory objects will have // type HistoryEntry - VKey contactHistoryVKey = - VKey.createOfy(ContactHistory.class, Key.create(contactHistory)); + VKey contactHistoryVKey = contactHistory.createVKey(); VKey historyEntryVKey = VKey.createOfy(HistoryEntry.class, Key.create(contactHistory.asHistoryEntry())); ContactHistory hostHistoryFromDb = tm().transact(() -> tm().load(contactHistoryVKey)); @@ -110,8 +110,7 @@ public class ContactHistoryTest extends EntityTestCase { assertThat(hostHistoryFromDb).isEqualTo(historyEntryFromDb); } - private ContactHistory createContactHistory( - ContactBase contact, VKey contactVKey) { + private ContactHistory createContactHistory(ContactBase contact, String contactRepoId) { return new ContactHistory.Builder() .setType(HistoryEntry.Type.HOST_CREATE) .setXmlBytes("".getBytes(UTF_8)) @@ -122,14 +121,14 @@ public class ContactHistoryTest extends EntityTestCase { .setReason("reason") .setRequestedByRegistrar(true) .setContactBase(contact) - .setContactRepoId(contactVKey) + .setContactRepoId(contactRepoId) .build(); } static void assertContactHistoriesEqual(ContactHistory one, ContactHistory two) { assertAboutImmutableObjects() .that(one) - .isEqualExceptFields(two, "contactBase", "contactRepoId", "parent"); + .isEqualExceptFields(two, "contactBase", "contactRepoId"); assertAboutImmutableObjects() .that(one.getContactBase().orElse(null)) .isEqualExceptFields(two.getContactBase().orElse(null), "repoId"); diff --git a/core/src/test/java/google/registry/model/history/DomainHistoryTest.java b/core/src/test/java/google/registry/model/history/DomainHistoryTest.java index c5a39c843..8265c001a 100644 --- a/core/src/test/java/google/registry/model/history/DomainHistoryTest.java +++ b/core/src/test/java/google/registry/model/history/DomainHistoryTest.java @@ -121,8 +121,7 @@ public class DomainHistoryTest extends EntityTestCase { // retrieving a HistoryEntry or a DomainHistory with the same key should return the same object // note: due to the @EntitySubclass annotation. all Keys for DomainHistory objects will have // type HistoryEntry - VKey domainHistoryVKey = - VKey.createOfy(DomainHistory.class, Key.create(domainHistory)); + VKey domainHistoryVKey = domainHistory.createVKey(); VKey historyEntryVKey = VKey.createOfy(HistoryEntry.class, Key.create(domainHistory.asHistoryEntry())); DomainHistory domainHistoryFromDb = tm().transact(() -> tm().load(domainHistoryVKey)); @@ -155,7 +154,7 @@ public class DomainHistoryTest extends EntityTestCase { assertAboutImmutableObjects() .that(one) .isEqualExceptFields( - two, "domainContent", "domainRepoId", "parent", "nsHosts", "domainTransactionRecords"); + two, "domainContent", "domainRepoId", "nsHosts", "domainTransactionRecords"); assertThat(one.getDomainContent().map(DomainContent::getDomainName)) .isEqualTo(two.getDomainContent().map(DomainContent::getDomainName)); // NB: the record's ID gets reset by Hibernate, causing the hash code to differ so we have to diff --git a/core/src/test/java/google/registry/model/history/HostHistoryTest.java b/core/src/test/java/google/registry/model/history/HostHistoryTest.java index ed0d8b88f..1fcf83a75 100644 --- a/core/src/test/java/google/registry/model/history/HostHistoryTest.java +++ b/core/src/test/java/google/registry/model/history/HostHistoryTest.java @@ -48,14 +48,13 @@ public class HostHistoryTest extends EntityTestCase { VKey hostVKey = VKey.create(HostResource.class, "host1", Key.create(HostResource.class, "host1")); HostResource hostFromDb = jpaTm().transact(() -> jpaTm().load(hostVKey)); - HostHistory hostHistory = createHostHistory(hostFromDb, hostVKey); + HostHistory hostHistory = createHostHistory(hostFromDb, host.getRepoId()); hostHistory.id = null; jpaTm().transact(() -> jpaTm().insert(hostHistory)); jpaTm() .transact( () -> { - HostHistory fromDatabase = - jpaTm().load(VKey.createSql(HostHistory.class, hostHistory.getId())); + HostHistory fromDatabase = jpaTm().load(hostHistory.createVKey()); assertHostHistoriesEqual(fromDatabase, hostHistory); assertThat(fromDatabase.getHostRepoId().getSqlKey()) .isEqualTo(hostHistory.getHostRepoId().getSqlKey()); @@ -68,20 +67,16 @@ public class HostHistoryTest extends EntityTestCase { HostResource host = newHostResourceWithRoid("ns1.example.com", "host1"); jpaTm().transact(() -> jpaTm().insert(host)); - VKey hostVKey = - VKey.create(HostResource.class, "host1", Key.create(HostResource.class, "host1")); - HostResource hostFromDb = jpaTm().transact(() -> jpaTm().load(hostVKey)); - + HostResource hostFromDb = jpaTm().transact(() -> jpaTm().load(host.createVKey())); HostHistory hostHistory = - createHostHistory(hostFromDb, hostVKey).asBuilder().setHostBase(null).build(); + createHostHistory(hostFromDb, host.getRepoId()).asBuilder().setHostBase(null).build(); hostHistory.id = null; jpaTm().transact(() -> jpaTm().insert(hostHistory)); jpaTm() .transact( () -> { - HostHistory fromDatabase = - jpaTm().load(VKey.createSql(HostHistory.class, hostHistory.getId())); + HostHistory fromDatabase = jpaTm().load(hostHistory.createVKey()); assertHostHistoriesEqual(fromDatabase, hostHistory); assertThat(fromDatabase.getHostRepoId().getSqlKey()) .isEqualTo(hostHistory.getHostRepoId().getSqlKey()); @@ -97,14 +92,14 @@ public class HostHistoryTest extends EntityTestCase { VKey hostVKey = VKey.create(HostResource.class, "host1", Key.create(HostResource.class, "host1")); HostResource hostFromDb = tm().transact(() -> tm().load(hostVKey)); - HostHistory hostHistory = createHostHistory(hostFromDb, hostVKey); + HostHistory hostHistory = createHostHistory(hostFromDb, host.getRepoId()); fakeClock.advanceOneMilli(); tm().transact(() -> tm().insert(hostHistory)); // retrieving a HistoryEntry or a HostHistory with the same key should return the same object // note: due to the @EntitySubclass annotation. all Keys for HostHistory objects will have // type HistoryEntry - VKey hostHistoryVKey = VKey.createOfy(HostHistory.class, Key.create(hostHistory)); + VKey hostHistoryVKey = hostHistory.createVKey(); VKey historyEntryVKey = VKey.createOfy(HistoryEntry.class, Key.create(hostHistory.asHistoryEntry())); HostHistory hostHistoryFromDb = tm().transact(() -> tm().load(hostHistoryVKey)); @@ -120,7 +115,7 @@ public class HostHistoryTest extends EntityTestCase { .isEqualExceptFields(two.getHostBase().orElse(null), "repoId"); } - private HostHistory createHostHistory(HostBase hostBase, VKey hostVKey) { + private HostHistory createHostHistory(HostBase hostBase, String hostRepoId) { return new HostHistory.Builder() .setType(HistoryEntry.Type.HOST_CREATE) .setXmlBytes("".getBytes(UTF_8)) @@ -131,7 +126,7 @@ public class HostHistoryTest extends EntityTestCase { .setReason("reason") .setRequestedByRegistrar(true) .setHostBase(hostBase) - .setHostRepoId(hostVKey) + .setHostRepoId(hostRepoId) .build(); } } diff --git a/core/src/test/java/google/registry/model/history/LegacyHistoryObjectTest.java b/core/src/test/java/google/registry/model/history/LegacyHistoryObjectTest.java index ac9b3fc0a..75bb6f534 100644 --- a/core/src/test/java/google/registry/model/history/LegacyHistoryObjectTest.java +++ b/core/src/test/java/google/registry/model/history/LegacyHistoryObjectTest.java @@ -88,11 +88,7 @@ public class LegacyHistoryObjectTest extends EntityTestCase { jpaTm().insert(legacyContactHistory); }); ContactHistory legacyHistoryFromSql = - jpaTm() - .transact( - () -> - jpaTm() - .load(VKey.createSql(ContactHistory.class, legacyContactHistory.getId()))); + jpaTm().transact(() -> jpaTm().load(legacyContactHistory.createVKey())); assertAboutImmutableObjects() .that(legacyContactHistory) .isEqualExceptFields(legacyHistoryFromSql); @@ -172,9 +168,7 @@ public class LegacyHistoryObjectTest extends EntityTestCase { jpaTm().insert(legacyHostHistory); }); HostHistory legacyHistoryFromSql = - jpaTm() - .transact( - () -> jpaTm().load(VKey.createSql(HostHistory.class, legacyHostHistory.getId()))); + jpaTm().transact(() -> jpaTm().load(legacyHostHistory.createVKey())); assertAboutImmutableObjects().that(legacyHostHistory).isEqualExceptFields(legacyHistoryFromSql); // can't compare hostRepoId directly since it doesn't save the ofy key in SQL assertThat(legacyHostHistory.getHostRepoId().getSqlKey()) diff --git a/core/src/test/resources/google/registry/model/schema.txt b/core/src/test/resources/google/registry/model/schema.txt index da603505d..ac6266317 100644 --- a/core/src/test/resources/google/registry/model/schema.txt +++ b/core/src/test/resources/google/registry/model/schema.txt @@ -131,9 +131,9 @@ class google.registry.model.contact.ContactHistory { google.registry.model.domain.Period period; google.registry.model.eppcommon.Trid trid; google.registry.model.reporting.HistoryEntry$Type type; - google.registry.persistence.VKey contactRepoId; java.lang.Boolean requestedByRegistrar; java.lang.String clientId; + java.lang.String contactRepoId; java.lang.String otherClientId; java.lang.String reason; java.util.Set domainTransactionRecords; @@ -401,9 +401,9 @@ class google.registry.model.host.HostHistory { google.registry.model.eppcommon.Trid trid; google.registry.model.host.HostBase hostBase; google.registry.model.reporting.HistoryEntry$Type type; - google.registry.persistence.VKey hostRepoId; java.lang.Boolean requestedByRegistrar; java.lang.String clientId; + java.lang.String hostRepoId; java.lang.String otherClientId; java.lang.String reason; java.util.Set domainTransactionRecords; diff --git a/db/src/main/resources/sql/flyway.txt b/db/src/main/resources/sql/flyway.txt index fbb5317bd..b314fecd6 100644 --- a/db/src/main/resources/sql/flyway.txt +++ b/db/src/main/resources/sql/flyway.txt @@ -56,3 +56,4 @@ V55__domain_history_fields.sql V56__rename_host_table.sql V57__history_null_content.sql V58__drop_default_value_and_sequences_for_billing_event.sql +V59__use_composite_primary_key_for_contact_and_host_history_table.sql diff --git a/db/src/main/resources/sql/flyway/V59__use_composite_primary_key_for_contact_and_host_history_table.sql b/db/src/main/resources/sql/flyway/V59__use_composite_primary_key_for_contact_and_host_history_table.sql new file mode 100644 index 000000000..5717a06ed --- /dev/null +++ b/db/src/main/resources/sql/flyway/V59__use_composite_primary_key_for_contact_and_host_history_table.sql @@ -0,0 +1,23 @@ +-- Copyright 2020 The Nomulus 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. + +alter table "ContactHistory" drop constraint "ContactHistory_pkey"; + +alter table "ContactHistory" + add constraint "ContactHistory_pkey" primary key (contact_repo_id, history_revision_id); + +alter table "HostHistory" drop constraint "HostHistory_pkey"; + +alter table "HostHistory" + add constraint "HostHistory_pkey" primary key (host_repo_id, history_revision_id); diff --git a/db/src/main/resources/sql/schema/db-schema.sql.generated b/db/src/main/resources/sql/schema/db-schema.sql.generated index 483291454..043de5415 100644 --- a/db/src/main/resources/sql/schema/db-schema.sql.generated +++ b/db/src/main/resources/sql/schema/db-schema.sql.generated @@ -151,7 +151,8 @@ create sequence temp_history_id_sequence start 1 increment 50; ); create table "ContactHistory" ( - history_revision_id int8 not null, + contact_repo_id text not null, + history_revision_id int8 not null, history_by_superuser boolean not null, history_registrar_id text, history_modification_time timestamptz not null, @@ -215,8 +216,7 @@ create sequence temp_history_id_sequence start 1 increment 50; last_epp_update_time timestamptz, statuses text[], update_timestamp timestamptz, - contact_repo_id text not null, - primary key (history_revision_id) + primary key (contact_repo_id, history_revision_id) ); create table "Cursor" ( @@ -400,7 +400,8 @@ create sequence temp_history_id_sequence start 1 increment 50; ); create table "HostHistory" ( - history_revision_id int8 not null, + host_repo_id text not null, + history_revision_id int8 not null, history_by_superuser boolean not null, history_registrar_id text, history_modification_time timestamptz not null, @@ -423,8 +424,7 @@ create sequence temp_history_id_sequence start 1 increment 50; last_epp_update_time timestamptz, statuses text[], update_timestamp timestamptz, - host_repo_id text not null, - primary key (history_revision_id) + primary key (host_repo_id, history_revision_id) ); create table "Lock" ( diff --git a/db/src/main/resources/sql/schema/nomulus.golden.sql b/db/src/main/resources/sql/schema/nomulus.golden.sql index 67a01ce27..78cf90583 100644 --- a/db/src/main/resources/sql/schema/nomulus.golden.sql +++ b/db/src/main/resources/sql/schema/nomulus.golden.sql @@ -1084,7 +1084,7 @@ ALTER TABLE ONLY public."ClaimsList" -- ALTER TABLE ONLY public."ContactHistory" - ADD CONSTRAINT "ContactHistory_pkey" PRIMARY KEY (history_revision_id); + ADD CONSTRAINT "ContactHistory_pkey" PRIMARY KEY (contact_repo_id, history_revision_id); -- @@ -1140,7 +1140,7 @@ ALTER TABLE ONLY public."GracePeriod" -- ALTER TABLE ONLY public."HostHistory" - ADD CONSTRAINT "HostHistory_pkey" PRIMARY KEY (history_revision_id); + ADD CONSTRAINT "HostHistory_pkey" PRIMARY KEY (host_repo_id, history_revision_id); --