zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-07-10 21:23:22 +02:00
Code Issues Projects Releases Packages Wiki Activity

Safely lazy load claims and reserved lists (#1177)

Browse source 
* Safely lazy load claims and reserved lists

This moves the entries of all of these lists into "insignificant" fields and
manages them explicitly.

* Additional fixes

Fix a few problems that came up in the merge or weren't caught in earlier
local test runs.

* Changes for review

- removed debug code
- added comments
- improved some methods that were loading the entire claims list
  unnecessarily.

* Fixed javadoc links

* Reformatted

* Minor fix for review
This commit is contained in:
Michael Muller 2021-05-25 11:28:30 -04:00 • committed by GitHub
parent 36b5f1b757
commit 5e8726b92b
13 changed files with 276 additions and 76 deletions
Download patch file Download diff file Expand all files Collapse all files

5
core/src/main/java/google/registry/model/registry/label/BaseDomainLabelList.java
View file

@ -38,7 +38,6 @@ import google.registry.model.ImmutableObject;
import google.registry.model.annotations.InCrossTld;
import google.registry.model.common.EntityGroupRoot;
import google.registry.model.registry.Registry;
import google.registry.model.registry.label.ReservedList.ReservedListEntry;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@ -56,8 +55,8 @@ import org.joda.time.DateTime;
* Base class for {@link ReservedList} and {@link PremiumList} objects stored in Datastore.
*
* @param <T> The type of the root value being listed, e.g. {@link ReservationType}.
* @param <R> The type of domain label entry being listed, e.g. {@link ReservedListEntry} (note,
* must subclass {@link DomainLabelEntry}.
* @param <R> The type of domain label entry being listed, e.g. {@link
* ReservedList.ReservedListEntry} (note, must subclass {@link DomainLabelEntry}.
*/
@MappedSuperclass
@InCrossTld

3
core/src/main/java/google/registry/model/registry/label/DomainLabelEntry.java
View file

@ -36,7 +36,8 @@ public abstract class DomainLabelEntry<T extends Comparable<?>, D extends Domain
extends ImmutableObject implements Comparable<D> {
@Id
@Column(name = "domain_label", insertable = false, updatable = false)
@javax.persistence.Id
@Column(name = "domain_label", nullable = false)
String label;
String comment;

83
core/src/main/java/google/registry/model/registry/label/ReservedList.java
View file

@ -17,9 +17,13 @@ package google.registry.model.registry.label;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Strings.isNullOrEmpty;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static google.registry.config.RegistryConfig.getDomainLabelListCacheDuration;
import static google.registry.model.ImmutableObject.Insignificant;
import static google.registry.model.registry.label.ReservationType.FULLY_BLOCKED;
import static google.registry.persistence.transaction.QueryComposer.Comparator.EQ;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import static google.registry.util.CollectionUtils.nullToEmpty;
import static org.joda.time.DateTimeZone.UTC;
@ -40,19 +44,19 @@ import google.registry.model.annotations.ReportedOn;
import google.registry.model.registry.Registry;
import google.registry.model.registry.label.DomainLabelMetrics.MetricsReservedListMatch;
import google.registry.schema.replay.NonReplicatedEntity;
import java.io.Serializable;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ExecutionException;
import javax.annotation.Nullable;
import javax.persistence.CollectionTable;
import javax.persistence.Column;
import javax.persistence.ElementCollection;
import javax.persistence.Embeddable;
import javax.persistence.Id;
import javax.persistence.Index;
import javax.persistence.JoinColumn;
import javax.persistence.MapKeyColumn;
import javax.persistence.PostPersist;
import javax.persistence.PreRemove;
import javax.persistence.Table;
import javax.persistence.Transient;
import org.joda.time.DateTime;
/**
@ -71,25 +75,60 @@ public final class ReservedList
extends BaseDomainLabelList<ReservationType, ReservedList.ReservedListEntry>
implements NonReplicatedEntity {
/**
* Mapping from domain name to its reserved list info.
*
* <p>This field requires special treatment since we want to lazy load it. We have to remove it
* from the immutability contract so we can modify it after construction and we have to handle the
* database processing on our own so we can detach it after load.
*/
@Mapify(ReservedListEntry.LabelMapper.class)
@ElementCollection
@CollectionTable(
name = "ReservedEntry",
joinColumns = @JoinColumn(name = "revisionId", referencedColumnName = "revisionId"))
@MapKeyColumn(name = "domain_label")
@Insignificant
@Transient
Map<String, ReservedListEntry> reservedListMap;
@Column(nullable = false)
boolean shouldPublish = true;
@PreRemove
void preRemove() {
jpaTm()
.query("DELETE FROM ReservedEntry WHERE revision_id = :revisionId")
.setParameter("revisionId", revisionId)
.executeUpdate();
}
/**
* Hibernate hook called on the insert of a new ReservedList. Stores the associated {@link
* ReservedEntry}'s.
*
* <p>We need to persist the list entries, but only on the initial insert (not on update) since
* the entries themselves never get changed, so we only annotate it with {@link PostPersist}, not
* {@link PostUpdate}.
*/
@PostPersist
void postPersist() {
if (reservedListMap != null) {
reservedListMap.values().stream()
.forEach(
entry -> {
// We can safely change the revision id since it's "Insignificant".
entry.revisionId = revisionId;
jpaTm().insert(entry);
});
}
}
/**
* A reserved list entry entity, persisted to Datastore, that represents a single label and its
* reservation type.
*/
@Embed
@Embeddable
@javax.persistence.Entity(name = "ReservedEntry")
public static class ReservedListEntry extends DomainLabelEntry<ReservationType, ReservedListEntry>
implements Buildable {
implements Buildable, NonReplicatedEntity, Serializable {
@Insignificant @Id Long revisionId;
@Column(nullable = false)
ReservationType reservationType;
@ -164,8 +203,24 @@ public final class ReservedList
return shouldPublish;
}
/** Returns a {@link Map} of domain labels to {@link ReservedListEntry}. */
public ImmutableMap<String, ReservedListEntry> getReservedListEntries() {
/**
* Returns a {@link Map} of domain labels to {@link ReservedListEntry}.
*
* <p>Note that this involves a database fetch of a potentially large number of elements and
* should be avoided unless necessary.
*/
public synchronized ImmutableMap<String, ReservedListEntry> getReservedListEntries() {
if (reservedListMap == null) {
reservedListMap =
jpaTm()
.transact(
() ->
jpaTm()
.createQueryComposer(ReservedListEntry.class)
.where("revisionId", EQ, revisionId)
.stream()
.collect(toImmutableMap(ReservedListEntry::getLabel, e -> e)));
}
return ImmutableMap.copyOf(nullToEmpty(reservedListMap));
}

5
core/src/main/java/google/registry/model/registry/label/ReservedListDao.java
View file

@ -52,9 +52,8 @@ public class ReservedListDao {
() ->
jpaTm()
.query(
"FROM ReservedList rl LEFT JOIN FETCH rl.reservedListMap WHERE"
+ " rl.revisionId IN (SELECT MAX(revisionId) FROM ReservedList subrl"
+ " WHERE subrl.name = :name)",
"FROM ReservedList WHERE revisionId IN "
+ "(SELECT MAX(revisionId) FROM ReservedList WHERE name = :name)",
ReservedList.class)
.setParameter("name", reservedListName)
.getResultStream()

56
core/src/main/java/google/registry/model/tmch/ClaimsEntry.java Normal file
View file

@ -0,0 +1,56 @@
// Copyright 2021 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.
package google.registry.model.tmch;
import google.registry.model.ImmutableObject;
import google.registry.schema.replay.NonReplicatedEntity;
import java.io.Serializable;
import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.Id;
/**
* Claims entry record, used by ClaimsList for persistence.
*
* <p>It would be preferable to have this nested in {@link ClaimsList}, but for some reason
* hibernate won't generate this into the schema in this case. We may not care, as we only use the
* generated schema for informational purposes and persistence against the actual schema seems to
* work.
*/
@Entity(name = "ClaimsEntry")
class ClaimsEntry extends ImmutableObject implements NonReplicatedEntity, Serializable {
@Id private Long revisionId;
@Id private String domainLabel;
@Column(nullable = false)
private String claimKey;
/** Default constructor for Hibernate. */
ClaimsEntry() {}
ClaimsEntry(Long revisionId, String domainLabel, String claimKey) {
this.revisionId = revisionId;
this.domainLabel = domainLabel;
this.claimKey = claimKey;
}
String getDomainLabel() {
return domainLabel;
}
String getClaimKey() {
return claimKey;
}
}

114
core/src/main/java/google/registry/model/tmch/ClaimsList.java
View file

@ -16,13 +16,15 @@ package google.registry.model.tmch;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static google.registry.model.ofy.ObjectifyService.allocateId;
import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import static google.registry.persistence.transaction.QueryComposer.Comparator.EQ;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
import com.googlecode.objectify.Key;
import com.googlecode.objectify.annotation.EmbedMap;
import com.googlecode.objectify.annotation.Entity;
import com.googlecode.objectify.annotation.Id;
import com.googlecode.objectify.annotation.Ignore;
@ -39,13 +41,11 @@ import google.registry.schema.replay.NonReplicatedEntity;
import java.util.Map;
import java.util.Optional;
import javax.annotation.Nullable;
import javax.persistence.CollectionTable;
import javax.persistence.Column;
import javax.persistence.ElementCollection;
import javax.persistence.GeneratedValue;
import javax.persistence.GenerationType;
import javax.persistence.JoinColumn;
import javax.persistence.MapKeyColumn;
import javax.persistence.PostPersist;
import javax.persistence.PreRemove;
import javax.persistence.Table;
import javax.persistence.Transient;
import org.joda.time.DateTime;
@ -94,15 +94,40 @@ public class ClaimsList extends ImmutableObject implements NonReplicatedEntity {
@Column(name = "tmdb_generation_time", nullable = false)
DateTime creationTime;
/** A map from labels to claims keys. */
@EmbedMap
@ElementCollection
@CollectionTable(
name = "ClaimsEntry",
joinColumns = @JoinColumn(name = "revisionId", referencedColumnName = "revisionId"))
@MapKeyColumn(name = "domainLabel", nullable = false)
@Column(name = "claimKey", nullable = false)
Map<String, String> labelsToKeys;
/**
* A map from labels to claims keys.
*
* <p>This field requires special treatment since we want to lazy load it. We have to remove it
* from the immutability contract so we can modify it after construction and we have to handle the
* database processing on our own so we can detach it after load.
*/
@Insignificant @Transient ImmutableMap<String, String> labelsToKeys;
@PreRemove
void preRemove() {
jpaTm()
.query("DELETE FROM ClaimsEntry WHERE revision_id = :revisionId")
.setParameter("revisionId", revisionId)
.executeUpdate();
}
/**
* Hibernate hook called on the insert of a new ReservedList. Stores the associated {@link
* ReservedListEntry}'s.
*
* <p>We need to persist the list entries, but only on the initial insert (not on update) since
* the entries themselves never get changed, so we only annotate it with {@link PostPersist}, not
* {@link PostUpdate}.
*/
@PostPersist
void postPersist() {
if (labelsToKeys != null) {
labelsToKeys.entrySet().stream()
.forEach(
entry ->
jpaTm().insert(new ClaimsEntry(revisionId, entry.getKey(), entry.getValue())));
}
}
/** Returns the revision id of this claims list, or throws exception if it is null. */
public Long getRevisionId() {
@ -126,22 +151,69 @@ public class ClaimsList extends ImmutableObject implements NonReplicatedEntity {
return creationTimestamp.getTimestamp();
}
/** Returns the claim key for a given domain if there is one, empty otherwise. */
/**
* Returns the claim key for a given domain if there is one, empty otherwise.
*
* <p>Note that this may do a database query. For checking multiple keys against the claims list
* it may be more efficient to use {@link #getLabelsToKeys()} first, as this will prefetch all
* entries and cache them locally.
*/
public Optional<String> getClaimKey(String label) {
return Optional.ofNullable(labelsToKeys.get(label));
if (labelsToKeys != null) {
return Optional.ofNullable(labelsToKeys.get(label));
}
return jpaTm()
.transact(
() ->
jpaTm()
.createQueryComposer(ClaimsEntry.class)
.where("revisionId", EQ, revisionId)
.where("domainLabel", EQ, label)
.first()
.map(ClaimsEntry::getClaimKey));
}
/** Returns an {@link Map} mapping domain label to its lookup key. */
/**
* Returns an {@link Map} mapping domain label to its lookup key.
*
* <p>Note that this involves a database fetch of a potentially large number of elements and
* should be avoided unless necessary.
*/
public ImmutableMap<String, String> getLabelsToKeys() {
return ImmutableMap.copyOf(labelsToKeys);
if (labelsToKeys == null) {
labelsToKeys =
jpaTm()
.transact(
() ->
jpaTm()
.createQueryComposer(ClaimsEntry.class)
.where("revisionId", EQ, revisionId)
.stream()
.collect(
toImmutableMap(
ClaimsEntry::getDomainLabel, ClaimsEntry::getClaimKey)));
}
return labelsToKeys;
}
/** Returns the number of claims. */
public int size() {
/**
* Returns the number of claims.
*
* <p>Note that this will perform a database "count" query if the label to key map has not been
* previously cached by calling {@link #getLabelsToKeys()}.
*/
public long size() {
if (labelsToKeys == null) {
return jpaTm()
.createQueryComposer(ClaimsEntry.class)
.where("revisionId", EQ, revisionId)
.count();
}
return labelsToKeys.size();
}
public static ClaimsList create(DateTime tmdbGenerationTime, Map<String, String> labelsToKeys) {
public static ClaimsList create(
DateTime tmdbGenerationTime, ImmutableMap<String, String> labelsToKeys) {
ClaimsList instance = new ClaimsList();
instance.id = allocateId();
instance.creationTime = checkNotNull(tmdbGenerationTime);

11
core/src/main/java/google/registry/model/tmch/ClaimsListDao.java
View file

@ -14,6 +14,7 @@
package google.registry.model.tmch;
import static google.registry.persistence.transaction.QueryComposer.Comparator.EQ;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import static google.registry.util.DateTimeUtils.START_OF_TIME;
@ -40,13 +41,9 @@ public class ClaimsListDao {
.query("SELECT MAX(revisionId) FROM ClaimsList", Long.class)
.getSingleResult();
return jpaTm()
.query(
"FROM ClaimsList cl LEFT JOIN FETCH cl.labelsToKeys WHERE cl.revisionId ="
+ " :revisionId",
ClaimsList.class)
.setParameter("revisionId", revisionId)
.getResultStream()
.findFirst();
.createQueryComposer(ClaimsList.class)
.where("revisionId", EQ, revisionId)
.first();
})
.orElse(ClaimsList.create(START_OF_TIME, ImmutableMap.of()));
}

25
core/src/main/java/google/registry/tools/CreateReservedListCommand.java
View file

@ -30,6 +30,7 @@ import google.registry.model.registry.label.ReservedList;
import google.registry.persistence.VKey;
import java.nio.file.Files;
import java.util.List;
import java.util.stream.Collectors;
import org.joda.time.DateTime;
/** Command to create a {@link ReservedList}. */
@ -73,6 +74,30 @@ final class CreateReservedListCommand extends CreateOrUpdateReservedListCommand
null, reservedList, VKey.createOfy(ReservedList.class, Key.create(reservedList)));
}
@Override
protected String prompt() {
return getChangedEntities().isEmpty()
? "No entity changes to apply."
: getChangedEntities().stream()
.map(
entity -> {
if (entity instanceof ReservedList) {
// Format the entries of the reserved list as well.
String entries =
((ReservedList) entity)
.getReservedListEntries().entrySet().stream()
.map(
entry ->
String.format("%s=%s", entry.getKey(), entry.getValue()))
.collect(Collectors.joining(", "));
return String.format("%s\nreservedListMap={%s}\n", entity, entries);
} else {
return entity.toString();
}
})
.collect(Collectors.joining("\n"));
}
private static void validateListName(String name) {
List<String> nameParts = Splitter.on('_').splitToList(name);
checkArgument(nameParts.size() == 2, INVALID_FORMAT_ERROR_MESSAGE);

2
core/src/main/resources/META-INF/persistence.xml
View file

@ -61,6 +61,7 @@
<class>google.registry.model.registrar.RegistrarContact</class>
<class>google.registry.model.registry.label.PremiumList</class>
<class>google.registry.model.registry.label.ReservedList</class>
<class>google.registry.model.registry.label.ReservedList$ReservedListEntry</class>
<class>google.registry.model.registry.Registry</class>
<class>google.registry.model.reporting.DomainTransactionRecord</class>
<class>google.registry.model.reporting.Spec11ThreatMatch</class>
@ -69,6 +70,7 @@
<class>google.registry.model.server.ServerSecret</class>
<class>google.registry.model.smd.SignedMarkRevocationList</class>
<class>google.registry.model.tmch.ClaimsList</class>
<class>google.registry.model.tmch.ClaimsEntry</class>
<class>google.registry.model.tmch.TmchCrl</class>
<class>google.registry.persistence.transaction.TransactionEntity</class>
<class>google.registry.schema.domain.RegistryLock</class>

24
core/src/test/java/google/registry/model/registry/label/ReservedListTest.java
View file

@ -24,6 +24,7 @@ import static google.registry.model.registry.label.DomainLabelMetrics.reservedLi
import static google.registry.model.registry.label.ReservationType.ALLOWED_IN_SUNRISE;
import static google.registry.model.registry.label.ReservationType.FULLY_BLOCKED;
import static google.registry.model.registry.label.ReservationType.NAME_COLLISION;
import static google.registry.model.registry.label.ReservedList.ReservedListEntry;
import static google.registry.model.registry.label.ReservedList.getReservationTypes;
import static google.registry.testing.DatabaseHelper.createTld;
import static google.registry.testing.DatabaseHelper.persistReservedList;
@ -34,32 +35,37 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import google.registry.model.ofy.Ofy;
import google.registry.model.registry.Registry;
import google.registry.model.registry.label.ReservedList.ReservedListEntry;
import google.registry.schema.tld.PremiumEntry;
import google.registry.testing.AppEngineExtension;
import google.registry.testing.DatabaseHelper;
import google.registry.testing.FakeClock;
import google.registry.testing.InjectExtension;
import org.joda.time.DateTime;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
/** Unit tests for {@link ReservedList}. */
class ReservedListTest {
@RegisterExtension final InjectExtension inject = new InjectExtension();
private FakeClock clock = new FakeClock(DateTime.parse("2010-01-01T10:00:00Z"));
@Order(value = Order.DEFAULT - 1)
@RegisterExtension
final InjectExtension inject =
new InjectExtension().withStaticFieldOverride(Ofy.class, "clock", clock);
@RegisterExtension
final AppEngineExtension appEngine =
AppEngineExtension.builder().withDatastoreAndCloudSql().build();
private FakeClock clock = new FakeClock(DateTime.parse("2010-01-01T10:00:00Z"));
AppEngineExtension.builder()
.withClock(clock)
.withJpaUnitTestEntities(
PremiumList.class, PremiumEntry.class, ReservedList.class, ReservedListEntry.class)
.withDatastoreAndCloudSql()
.build();
@BeforeEach
void beforeEach() {
inject.setStaticField(Ofy.class, "clock", clock);
// Auto-increment clock in DatabaseHelper.
inject.setStaticField(DatabaseHelper.class, "clock", clock);
createTld("tld");
reservedListChecks.reset();
reservedListProcessingTime.reset();

5
core/src/test/java/google/registry/tools/CreateOrUpdateReservedListCommandTestCase.java
View file

@ -125,10 +125,7 @@ abstract class CreateOrUpdateReservedListCommandTestCase<
.setParameter("name", name)
.getSingleResult();
return jpaTm()
.query(
"FROM ReservedList rl LEFT JOIN FETCH rl.reservedListMap WHERE"
+ " rl.revisionId = :revisionId",
ReservedList.class)
.query("FROM ReservedList WHERE revisionId = :revisionId", ReservedList.class)
.setParameter("revisionId", revisionId)
.getSingleResult();
});

3
core/src/test/resources/google/registry/model/schema.txt
View file

@ -754,6 +754,7 @@ class google.registry.model.registry.label.ReservedList {
class google.registry.model.registry.label.ReservedList$ReservedListEntry {
@Id java.lang.String label;
google.registry.model.registry.label.ReservationType reservationType;
java.lang.Long revisionId;
java.lang.String comment;
}
class google.registry.model.reporting.DomainTransactionRecord {
@ -862,7 +863,7 @@ class google.registry.model.server.ServerSecret {
class google.registry.model.tmch.ClaimsList {
@Id long id;
@Parent com.googlecode.objectify.Key<google.registry.model.tmch.ClaimsList$ClaimsListRevision> parent;
java.util.Map<java.lang.String, java.lang.String> labelsToKeys;
com.google.common.collect.ImmutableMap<java.lang.String, java.lang.String> labelsToKeys;
org.joda.time.DateTime creationTime;
}
class google.registry.model.tmch.ClaimsList$ClaimsListRevision {

16
db/src/main/resources/sql/schema/db-schema.sql.generated
View file

@ -83,8 +83,8 @@
create table "ClaimsEntry" (
revision_id int8 not null,
claim_key text not null,
domain_label text not null,
claim_key text not null,
primary key (revision_id, domain_label)
);
@ -650,9 +650,9 @@
create table "ReservedEntry" (
revision_id int8 not null,
reservation_type int4 not null,
comment text,
domain_label text not null,
comment text,
reservation_type int4 not null,
primary key (revision_id, domain_label)
);
@ -822,11 +822,6 @@ create index spec11threatmatch_registrar_id_idx on "Spec11ThreatMatch" (registra
create index spec11threatmatch_tld_idx on "Spec11ThreatMatch" (tld);
create index spec11threatmatch_check_date_idx on "Spec11ThreatMatch" (check_date);
alter table if exists "ClaimsEntry"
add constraint FK6sc6at5hedffc0nhdcab6ivuq
foreign key (revision_id)
references "ClaimsList";
alter table if exists "DelegationSignerData"
add constraint FKtr24j9v14ph2mfuw2gsmt12kq
foreign key (domain_repo_id)
@ -867,11 +862,6 @@ create index spec11threatmatch_check_date_idx on "Spec11ThreatMatch" (check_date
foreign key (relock_revision_id)
references "RegistryLock";
alter table if exists "ReservedEntry"
add constraint FKgq03rk0bt1hb915dnyvd3vnfc
foreign key (revision_id)
references "ReservedList";
alter table if exists "SignedMarkRevocationEntry"
add constraint FK5ivlhvs3121yx2li5tqh54u4
foreign key (revision_id)