diff --git a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java index e55153d3d..142306c03 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java @@ -67,6 +67,7 @@ import google.registry.model.domain.DomainCommand.Update.AddRemove; import google.registry.model.domain.DomainCommand.Update.Change; import google.registry.model.domain.fee.FeeUpdateCommandExtension; import google.registry.model.domain.metadata.MetadataExtension; +import google.registry.model.domain.secdns.DelegationSignerData; import google.registry.model.domain.secdns.SecDnsUpdateExtension; import google.registry.model.domain.superuser.DomainUpdateSuperuserExtension; import google.registry.model.eppcommon.AuthInfo; @@ -238,10 +239,16 @@ public final class DomainUpdateFlow implements TransactionalFlow { DomainBase.Builder domainBuilder = domain .asBuilder() - // Handle the secDNS extension. + // Handle the secDNS extension. As dsData in secDnsUpdate is read from EPP input and + // does not have domainRepoId set, we create a copy of the existing dsData without + // domainRepoId for comparison. .setDsData( secDnsUpdate.isPresent() - ? updateDsData(domain.getDsData(), secDnsUpdate.get()) + ? updateDsData( + domain.getDsData().stream() + .map(DelegationSignerData::cloneWithoutDomainRepoId) + .collect(toImmutableSet()), + secDnsUpdate.get()) : domain.getDsData()) .setLastEppUpdateTime(now) .setLastEppUpdateClientId(clientId) diff --git a/core/src/main/java/google/registry/model/domain/DomainBase.java b/core/src/main/java/google/registry/model/domain/DomainBase.java index 5ac77edc5..67778758b 100644 --- a/core/src/main/java/google/registry/model/domain/DomainBase.java +++ b/core/src/main/java/google/registry/model/domain/DomainBase.java @@ -19,6 +19,7 @@ import google.registry.model.EppResource; import google.registry.model.EppResource.ForeignKeyedEppResource; import google.registry.model.annotations.ExternalMessagingName; import google.registry.model.annotations.ReportedOn; +import google.registry.model.domain.secdns.DelegationSignerData; import google.registry.model.host.HostResource; import google.registry.persistence.VKey; import google.registry.persistence.WithStringVKey; @@ -113,6 +114,32 @@ public class DomainBase extends DomainContent restoreOfyKeys(getRepoId()); } + /** + * Returns the set of {@link DelegationSignerData} associated with the domain. + * + *

This is the getter method specific for Hibernate to access the field so it is set to + * private. The caller can use the public {@link #getDsData()} to get the DS data. + * + *

Note that we need to set `insertable = false, updatable = false` for @JoinColumn, otherwise + * Hibernate would try to set the foreign key to null(through an UPDATE TABLE sql) instead of + * deleting the whole entry from the table when the {@link DelegationSignerData} is removed from + * the set. + */ + @Access(AccessType.PROPERTY) + @OneToMany( + cascade = {CascadeType.ALL}, + fetch = FetchType.EAGER, + orphanRemoval = true) + @JoinColumn( + name = "domainRepoId", + referencedColumnName = "repoId", + insertable = false, + updatable = false) + @SuppressWarnings("UnusedMethod") + private Set getInternalDelegationSignerData() { + return dsData; + } + @Override public VKey createVKey() { return VKey.create(DomainBase.class, getRepoId(), Key.create(this)); diff --git a/core/src/main/java/google/registry/model/domain/DomainContent.java b/core/src/main/java/google/registry/model/domain/DomainContent.java index bc73a9ff8..a726e1222 100644 --- a/core/src/main/java/google/registry/model/domain/DomainContent.java +++ b/core/src/main/java/google/registry/model/domain/DomainContent.java @@ -317,6 +317,10 @@ public class DomainContent extends EppResource autorenewPollMessageHistoryId = getHistoryId(autorenewPollMessage); autorenewBillingEventHistoryId = getHistoryId(autorenewBillingEvent); deletePollMessageHistoryId = getHistoryId(deletePollMessage); + dsData = + nullToEmptyImmutableCopy(dsData).stream() + .map(dsData -> dsData.cloneWithDomainRepoId(getRepoId())) + .collect(toImmutableSet()); } /** @@ -469,6 +473,12 @@ public class DomainContent extends EppResource this.gracePeriods = gracePeriods; } + // Hibernate needs this in order to populate dsData but no one else should ever use it + @SuppressWarnings("UnusedMethod") + private void setInternalDelegationSignerData(Set dsData) { + this.dsData = dsData; + } + public final String getCurrentSponsorClientId() { return getPersistedCurrentSponsorClientId(); } @@ -791,10 +801,16 @@ public class DomainContent extends EppResource instance.tld = getTldFromDomainName(instance.fullyQualifiedDomainName); T newDomain = super.build(); - // Hibernate throws exception if gracePeriods is null because we enabled all cascadable - // operations and orphan removal. + // Hibernate throws exception if gracePeriods or dsData is null because we enabled all + // cascadable operations and orphan removal. newDomain.gracePeriods = newDomain.gracePeriods == null ? ImmutableSet.of() : newDomain.gracePeriods; + newDomain.dsData = + newDomain.dsData == null + ? ImmutableSet.of() + : newDomain.dsData.stream() + .map(ds -> ds.cloneWithDomainRepoId(instance.getRepoId())) + .collect(toImmutableSet()); return newDomain; } diff --git a/core/src/main/java/google/registry/model/domain/secdns/DelegationSignerData.java b/core/src/main/java/google/registry/model/domain/secdns/DelegationSignerData.java index 1b4e0ea53..b7689d4a2 100644 --- a/core/src/main/java/google/registry/model/domain/secdns/DelegationSignerData.java +++ b/core/src/main/java/google/registry/model/domain/secdns/DelegationSignerData.java @@ -14,11 +14,22 @@ package google.registry.model.domain.secdns; +import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; + import com.googlecode.objectify.annotation.Embed; +import com.googlecode.objectify.annotation.Ignore; import google.registry.model.ImmutableObject; +import google.registry.model.domain.secdns.DelegationSignerData.DelegationSignerDataId; import google.registry.schema.replay.DatastoreAndSqlEntity; +import java.io.Serializable; +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.IdClass; +import javax.persistence.Index; +import javax.persistence.Table; import javax.xml.bind.DatatypeConverter; import javax.xml.bind.annotation.XmlElement; +import javax.xml.bind.annotation.XmlTransient; import javax.xml.bind.annotation.XmlType; import javax.xml.bind.annotation.adapters.HexBinaryAdapter; import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter; @@ -31,19 +42,26 @@ import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter; */ @Embed @XmlType(name = "dsData") -@javax.persistence.Entity +@Entity +@Table(indexes = @Index(columnList = "domainRepoId")) +@IdClass(DelegationSignerDataId.class) public class DelegationSignerData extends ImmutableObject implements DatastoreAndSqlEntity { private DelegationSignerData() {} + @Ignore @XmlTransient @javax.persistence.Id String domainRepoId; + /** The identifier for this particular key in the domain. */ - @javax.persistence.Id int keyTag; + @javax.persistence.Id + @Column(nullable = false) + int keyTag; /** * The algorithm used by this key. * * @see RFC 4034 Appendix A.1 */ + @Column(nullable = false) @XmlElement(name = "alg") int algorithm; @@ -52,6 +70,7 @@ public class DelegationSignerData extends ImmutableObject implements DatastoreAn * * @see RFC 4034 Appendix A.2 */ + @Column(nullable = false) int digestType; /** @@ -59,6 +78,7 @@ public class DelegationSignerData extends ImmutableObject implements DatastoreAn * * @see RFC 4034 Section 5.1.4 */ + @Column(nullable = false) @XmlJavaTypeAdapter(HexBinaryAdapter.class) byte[] digest; @@ -82,16 +102,34 @@ public class DelegationSignerData extends ImmutableObject implements DatastoreAn return digest == null ? "" : DatatypeConverter.printHexBinary(digest); } + public DelegationSignerData cloneWithDomainRepoId(String domainRepoId) { + DelegationSignerData clone = clone(this); + clone.domainRepoId = checkArgumentNotNull(domainRepoId); + return clone; + } + + public DelegationSignerData cloneWithoutDomainRepoId() { + DelegationSignerData clone = clone(this); + clone.domainRepoId = null; + return clone; + } + public static DelegationSignerData create( - int keyTag, int algorithm, int digestType, byte[] digest) { + int keyTag, int algorithm, int digestType, byte[] digest, String domainRepoId) { DelegationSignerData instance = new DelegationSignerData(); instance.keyTag = keyTag; instance.algorithm = algorithm; instance.digestType = digestType; instance.digest = digest; + instance.domainRepoId = domainRepoId; return instance; } + public static DelegationSignerData create( + int keyTag, int algorithm, int digestType, byte[] digest) { + return create(keyTag, algorithm, digestType, digest, null); + } + public static DelegationSignerData create( int keyTag, int algorithm, int digestType, String digestAsHex) { return create(keyTag, algorithm, digestType, DatatypeConverter.parseHexBinary(digestAsHex)); @@ -107,4 +145,20 @@ public class DelegationSignerData extends ImmutableObject implements DatastoreAn "%d %d %d %s", this.keyTag, this.algorithm, this.digestType, DatatypeConverter.printHexBinary(digest)); } + + static class DelegationSignerDataId extends ImmutableObject implements Serializable { + String domainRepoId; + int keyTag; + + private DelegationSignerDataId() {} + + private DelegationSignerDataId(String domainRepoId, int keyTag) { + this.domainRepoId = domainRepoId; + this.keyTag = keyTag; + } + + public static DelegationSignerDataId create(String domainRepoId, int keyTag) { + return new DelegationSignerDataId(checkArgumentNotNull(domainRepoId), keyTag); + } + } } diff --git a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java index 9e8b75581..3dbf750bd 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java @@ -821,10 +821,12 @@ class DomainCreateFlowTest extends ResourceFlowTestCase ds.cloneWithDomainRepoId(resource.getRepoId())) + .collect(toImmutableSet())); assertDnsTasksEnqueued("example.tld"); } diff --git a/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java b/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java index 08295aee1..e64a4caf0 100644 --- a/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java +++ b/core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java @@ -25,6 +25,7 @@ import static org.joda.time.DateTimeZone.UTC; import static org.junit.jupiter.api.Assertions.fail; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; import com.googlecode.objectify.Key; import google.registry.model.billing.BillingEvent; import google.registry.model.billing.BillingEvent.Flag; @@ -342,6 +343,50 @@ public class DomainBaseSqlTest { }); } + @Test + void testModifyDsData_addThenRemoveSuccessfully() { + persistDomain(); + DelegationSignerData extraDsData = + DelegationSignerData.create(2, 2, 3, new byte[] {0, 1, 2}, "4-COM"); + ImmutableSet unionDsData = + Sets.union(domain.getDsData(), ImmutableSet.of(extraDsData)).immutableCopy(); + + // Add an extra DelegationSignerData to dsData set. + jpaTm() + .transact( + () -> { + DomainBase persisted = jpaTm().load(domain.createVKey()); + assertThat(persisted.getDsData()).containsExactlyElementsIn(domain.getDsData()); + DomainBase modified = persisted.asBuilder().setDsData(unionDsData).build(); + jpaTm().put(modified); + }); + + // Verify that the persisted domain entity contains both DelegationSignerData records. + jpaTm() + .transact( + () -> { + DomainBase persisted = jpaTm().load(domain.createVKey()); + assertThat(persisted.getDsData()).containsExactlyElementsIn(unionDsData); + assertEqualDomainExcept(persisted, "dsData"); + }); + + // Remove the extra DelegationSignerData record from dsData set. + jpaTm() + .transact( + () -> { + DomainBase persisted = jpaTm().load(domain.createVKey()); + jpaTm().put(persisted.asBuilder().setDsData(domain.getDsData()).build()); + }); + + // Verify that the persisted domain is equal to the original domain. + jpaTm() + .transact( + () -> { + DomainBase persisted = jpaTm().load(domain.createVKey()); + assertEqualDomainExcept(persisted); + }); + } + @Test void testUpdates() { jpaTm() @@ -358,16 +403,6 @@ public class DomainBaseSqlTest { .transact( () -> { DomainBase result = jpaTm().load(domain.createVKey()); - - // Fix DS data, since we can't persist that yet. - result = - result - .asBuilder() - .setDsData( - ImmutableSet.of( - DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2}))) - .build(); - assertAboutImmutableObjects() .that(result) .isEqualExceptFields(domain, "updateTimestamp", "creationTime"); @@ -572,13 +607,6 @@ public class DomainBaseSqlTest { } private void assertEqualDomainExcept(DomainBase thatDomain, String... excepts) { - // Fix DS data, since we can't persist it yet. - thatDomain = - thatDomain - .asBuilder() - .setDsData(ImmutableSet.of(DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2}))) - .build(); - // Fix the original creation timestamp (this gets initialized on first write) DomainBase org = domain.asBuilder().setCreationTime(thatDomain.getCreationTime()).build(); diff --git a/db/src/main/resources/sql/flyway.txt b/db/src/main/resources/sql/flyway.txt index c87b8ddae..f459511b2 100644 --- a/db/src/main/resources/sql/flyway.txt +++ b/db/src/main/resources/sql/flyway.txt @@ -60,3 +60,4 @@ V59__use_composite_primary_key_for_contact_and_host_history_table.sql V60__remove_pollmessage_sequence.sql V61__domain_hist_columns.sql V62__disable_key_auto_generation_for_history_tables.sql +V63__add_schema_for_ds_data.sql diff --git a/db/src/main/resources/sql/flyway/V63__add_schema_for_ds_data.sql b/db/src/main/resources/sql/flyway/V63__add_schema_for_ds_data.sql new file mode 100644 index 000000000..18564dde1 --- /dev/null +++ b/db/src/main/resources/sql/flyway/V63__add_schema_for_ds_data.sql @@ -0,0 +1,29 @@ +-- 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. + +create table "DelegationSignerData" ( + domain_repo_id text not null, + key_tag int4 not null, + algorithm int4 not null, + digest bytea not null, + digest_type int4 not null, + primary key (domain_repo_id, key_tag) +); + +create index IDXhlqqd5uy98cjyos72d81x9j95 on "DelegationSignerData" (domain_repo_id); + +alter table if exists "DelegationSignerData" + add constraint FKtr24j9v14ph2mfuw2gsmt12kq + foreign key (domain_repo_id) + references "Domain"; 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 321f1b3de..c3cf3fae1 100644 --- a/db/src/main/resources/sql/schema/db-schema.sql.generated +++ b/db/src/main/resources/sql/schema/db-schema.sql.generated @@ -227,11 +227,12 @@ ); create table "DelegationSignerData" ( - key_tag int4 not null, + domain_repo_id text not null, + key_tag int4 not null, algorithm int4 not null, - digest bytea, + digest bytea not null, digest_type int4 not null, - primary key (key_tag) + primary key (domain_repo_id, key_tag) ); create table "Domain" ( @@ -668,6 +669,7 @@ create index IDXo1xdtpij2yryh0skxe9v91sep on "ContactHistory" (creation_time); create index IDXhp33wybmb6tbpr1bq7ttwk8je on "ContactHistory" (history_registrar_id); create index IDX9q53px6r302ftgisqifmc6put on "ContactHistory" (history_type); create index IDXsudwswtwqnfnx2o1hx4s0k0g5 on "ContactHistory" (history_modification_time); +create index IDXhlqqd5uy98cjyos72d81x9j95 on "DelegationSignerData" (domain_repo_id); create index IDX8nr0ke9mrrx4ewj6pd2ag4rmr on "Domain" (creation_time); create index IDXhsjqiy2lyobfymplb28nm74lm on "Domain" (current_sponsor_registrar_id); create index IDX5mnf0wn20tno4b9do88j61klr on "Domain" (deletion_time); @@ -705,6 +707,11 @@ create index spec11threatmatch_check_date_idx on "Spec11ThreatMatch" (check_date foreign key (revision_id) references "ClaimsList"; + alter table if exists "DelegationSignerData" + add constraint FKtr24j9v14ph2mfuw2gsmt12kq + foreign key (domain_repo_id) + references "Domain"; + alter table if exists "DomainHistoryHost" add constraint FKa9woh3hu8gx5x0vly6bai327n foreign key (domain_history_domain_repo_id, domain_history_history_revision_id) diff --git a/db/src/main/resources/sql/schema/nomulus.golden.sql b/db/src/main/resources/sql/schema/nomulus.golden.sql index 15f19d8dc..1cf1db30b 100644 --- a/db/src/main/resources/sql/schema/nomulus.golden.sql +++ b/db/src/main/resources/sql/schema/nomulus.golden.sql @@ -303,6 +303,19 @@ CREATE TABLE public."Cursor" ( ); +-- +-- Name: DelegationSignerData; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public."DelegationSignerData" ( + domain_repo_id text NOT NULL, + key_tag integer NOT NULL, + algorithm integer NOT NULL, + digest bytea NOT NULL, + digest_type integer NOT NULL +); + + -- -- Name: Domain; Type: TABLE; Schema: public; Owner: - -- @@ -1059,6 +1072,14 @@ ALTER TABLE ONLY public."Cursor" ADD CONSTRAINT "Cursor_pkey" PRIMARY KEY (scope, type); +-- +-- Name: DelegationSignerData DelegationSignerData_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public."DelegationSignerData" + ADD CONSTRAINT "DelegationSignerData_pkey" PRIMARY KEY (domain_repo_id, key_tag); + + -- -- Name: DomainHistory DomainHistory_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -1372,6 +1393,13 @@ CREATE INDEX idxeokttmxtpq2hohcioe5t2242b ON public."BillingCancellation" USING CREATE INDEX idxfg2nnjlujxo6cb9fha971bq2n ON public."HostHistory" USING btree (creation_time); +-- +-- Name: idxhlqqd5uy98cjyos72d81x9j95; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX idxhlqqd5uy98cjyos72d81x9j95 ON public."DelegationSignerData" USING btree (domain_repo_id); + + -- -- Name: idxhmv411mdqo5ibn4vy7ykxpmlv; Type: INDEX; Schema: public; Owner: - -- @@ -1985,6 +2013,14 @@ ALTER TABLE ONLY public."PremiumEntry" ADD CONSTRAINT fko0gw90lpo1tuee56l0nb6y6g5 FOREIGN KEY (revision_id) REFERENCES public."PremiumList"(revision_id); +-- +-- Name: DelegationSignerData fktr24j9v14ph2mfuw2gsmt12kq; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public."DelegationSignerData" + ADD CONSTRAINT fktr24j9v14ph2mfuw2gsmt12kq FOREIGN KEY (domain_repo_id) REFERENCES public."Domain"(repo_id); + + -- -- PostgreSQL database dump complete --