mirrors/nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus synced 2026-09-21 07:24:26 +00:00
Code Issues Releases Wiki Activity

Modify the way we load resources via foreign keys (#2852)

Browse Source 
Previously, we would have separate database calls for mapping from
foreign key to repo ID and then from repo ID to object. This PR modifies
those calls to load the resource directly (the old system was an
artifact of the Datastore key-value storage system).

In this PR, we merge the load-resource-by-foreign-key calls into a
single database load, as well as adding a separate cache object for
(foreign key) -> (resource). Now we cache, and have separate cleaner
code paths, for fk -> resource, fk -> repo ID, and repo ID -> resource.

Also removes the unused RdeFragmenter class
This commit is contained in:
gbrodman
2025-10-29 19:21:27 +00:00
committed by GitHub
parent 0c74883428
commit d9349be18e
35 changed files with 373 additions and 466 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/test/java/google/registry/bsa/BsaValidateActionTest.java
+2 -2
View File
@@ -239,7 +239,7 @@ public class BsaValidateActionTest {
persistBsaLabel("label");
Domain domain = persistActiveDomain("label.app", fakeClock.nowUtc());
fakeClock.advanceBy(MAX_STALENESS.minus(Duration.standardSeconds(1)));
assertThat(action.isStalenessAllowed(domain.createVKey())).isTrue();
assertThat(action.isStalenessAllowed(domain)).isTrue();
}
@Test
@@ -247,7 +247,7 @@ public class BsaValidateActionTest {
persistBsaLabel("label");
Domain domain = persistActiveDomain("label.app", fakeClock.nowUtc());
fakeClock.advanceBy(MAX_STALENESS);
assertThat(action.isStalenessAllowed(domain.createVKey())).isFalse();
assertThat(action.isStalenessAllowed(domain)).isFalse();
}
@Test
core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java
+1 -5
View File
@@ -752,11 +752,7 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
persistResource(
DatabaseHelper.newDomain("example1.tld")
.asBuilder()
.setRegistrant(
Optional.of(
ForeignKeyUtils.loadResource(Contact.class, "sh8013", clock.nowUtc())
.get()
.createVKey()))
.setRegistrant(ForeignKeyUtils.loadKey(Contact.class, "sh8013", clock.nowUtc()))
.setNameservers(ImmutableSet.of(host.createVKey()))
.setDeletionTime(START_OF_TIME)
.build());
core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java
+1 -3
View File
@@ -76,7 +76,6 @@ import google.registry.model.reporting.HistoryEntry;
import google.registry.model.tld.Tld;
import google.registry.model.transfer.DomainTransferData;
import google.registry.model.transfer.TransferStatus;
import google.registry.persistence.VKey;
import google.registry.testing.CloudTasksHelper.TaskMatcher;
import google.registry.testing.DatabaseHelper;
import javax.annotation.Nullable;
@@ -185,8 +184,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Host> {
Host renamedHost = doSuccessfulTest();
assertThat(renamedHost.isSubordinate()).isTrue();
assertHostDnsRequests("ns1.example.tld", "ns2.example.tld");
VKey<Host> oldVKeyAfterRename = ForeignKeyUtils.load(Host.class, oldHostName(), clock.nowUtc());
assertThat(oldVKeyAfterRename).isNull();
assertThat(ForeignKeyUtils.loadKey(Host.class, oldHostName(), clock.nowUtc())).isEmpty();
}
@Test
core/src/test/java/google/registry/model/ForeignKeyUtilsTest.java
+30 -24
View File
@@ -48,7 +48,10 @@ class ForeignKeyUtilsTest {
@RegisterExtension
public final TestCacheExtension testCacheExtension =
new TestCacheExtension.Builder().withForeignKeyCache(Duration.ofDays(1)).build();
new TestCacheExtension.Builder()
.withForeignKeyRepoIdCache(Duration.ofDays(1))
.withForeignKeyResourceCache(Duration.ofDays(1))
.build();
@BeforeEach
void setUp() {
@@ -56,46 +59,48 @@ class ForeignKeyUtilsTest {
}
@Test
void testSuccess_loadHost() {
void testSuccess_loadHostKey() {
Host host = persistActiveHost("ns1.example.com");
assertThat(ForeignKeyUtils.load(Host.class, "ns1.example.com", fakeClock.nowUtc()))
.isEqualTo(host.createVKey());
assertThat(ForeignKeyUtils.loadKey(Host.class, "ns1.example.com", fakeClock.nowUtc()))
.hasValue(host.createVKey());
}
@Test
void testSuccess_loadDomain() {
void testSuccess_loadDomainKey() {
Domain domain = persistActiveDomain("example.com");
assertThat(ForeignKeyUtils.load(Domain.class, "example.com", fakeClock.nowUtc()))
.isEqualTo(domain.createVKey());
assertThat(ForeignKeyUtils.loadKey(Domain.class, "example.com", fakeClock.nowUtc()))
.hasValue(domain.createVKey());
}
@Test
void testSuccess_loadContact() {
void testSuccess_loadContactKey() {
Contact contact = persistActiveContact("john-doe");
assertThat(ForeignKeyUtils.load(Contact.class, "john-doe", fakeClock.nowUtc()))
.isEqualTo(contact.createVKey());
assertThat(ForeignKeyUtils.loadKey(Contact.class, "john-doe", fakeClock.nowUtc()))
.hasValue(contact.createVKey());
}
@Test
void testSuccess_loadMostRecentResource() {
void testSuccess_loadKeyMostRecentResource() {
Host host = persistActiveHost("ns1.example.com");
persistResource(host.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build());
fakeClock.advanceOneMilli();
Host newHost = persistActiveHost("ns1.example.com");
assertThat(ForeignKeyUtils.load(Host.class, "ns1.example.com", fakeClock.nowUtc()))
.isEqualTo(newHost.createVKey());
assertThat(ForeignKeyUtils.loadKey(Host.class, "ns1.example.com", fakeClock.nowUtc()))
.hasValue(newHost.createVKey());
}
@Test
void testSuccess_loadNonexistentForeignKey_returnsNull() {
assertThat(ForeignKeyUtils.load(Host.class, "ns1.example.com", fakeClock.nowUtc())).isNull();
void testSuccess_loadKeyNonexistentForeignKey_returnsNull() {
assertThat(ForeignKeyUtils.loadKey(Host.class, "ns1.example.com", fakeClock.nowUtc()))
.isEmpty();
}
@Test
void testSuccess_loadDeletedForeignKey_returnsNull() {
void testSuccess_loadKeyDeletedForeignKey_returnsNull() {
Host host = persistActiveHost("ns1.example.com");
persistResource(host.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build());
assertThat(ForeignKeyUtils.load(Host.class, "ns1.example.com", fakeClock.nowUtc())).isNull();
assertThat(ForeignKeyUtils.loadKey(Host.class, "ns1.example.com", fakeClock.nowUtc()))
.isEmpty();
}
@Test
@@ -103,16 +108,17 @@ class ForeignKeyUtilsTest {
Host host1 = persistActiveHost("ns1.example.com");
fakeClock.advanceOneMilli();
persistResource(host1.asBuilder().setDeletionTime(fakeClock.nowUtc()).build());
assertThat(ForeignKeyUtils.load(Host.class, "ns1.example.com", fakeClock.nowUtc())).isNull();
assertThat(ForeignKeyUtils.loadKey(Host.class, "ns1.example.com", fakeClock.nowUtc()))
.isEmpty();
}
@Test
void testSuccess_batchLoad_skipsDeletedAndNonexistent() {
void testSuccess_batchLoadKeys_skipsDeletedAndNonexistent() {
Host host1 = persistActiveHost("ns1.example.com");
Host host2 = persistActiveHost("ns2.example.com");
persistResource(host2.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build());
assertThat(
ForeignKeyUtils.load(
ForeignKeyUtils.loadKeys(
Host.class,
ImmutableList.of("ns1.example.com", "ns2.example.com", "ns3.example.com"),
fakeClock.nowUtc()))
@@ -121,7 +127,7 @@ class ForeignKeyUtilsTest {
fakeClock.advanceOneMilli();
Host newHost1 = persistActiveHost("ns1.example.com");
assertThat(
ForeignKeyUtils.loadByCacheIfEnabled(
ForeignKeyUtils.loadKeysByCacheIfEnabled(
Host.class,
ImmutableList.of("ns1.example.com", "ns2.example.com", "ns3.example.com"),
fakeClock.nowUtc()))
@@ -129,12 +135,12 @@ class ForeignKeyUtilsTest {
}
@Test
void testSuccess_loadHostsCached_cacheIsStale() {
void testSuccess_loadHostKeysCached_cacheIsStale() {
Host host1 = persistActiveHost("ns1.example.com");
Host host2 = persistActiveHost("ns2.example.com");
persistResource(host2.asBuilder().setDeletionTime(fakeClock.nowUtc().minusDays(1)).build());
assertThat(
ForeignKeyUtils.loadByCacheIfEnabled(
ForeignKeyUtils.loadKeysByCacheIfEnabled(
Host.class,
ImmutableList.of("ns1.example.com", "ns2.example.com", "ns3.example.com"),
fakeClock.nowUtc()))
@@ -144,7 +150,7 @@ class ForeignKeyUtilsTest {
persistActiveHost("ns1.example.com");
// Even though a new host1 is now live, the cache still returns the VKey to the old one.
assertThat(
ForeignKeyUtils.loadByCacheIfEnabled(
ForeignKeyUtils.loadKeysByCacheIfEnabled(
Host.class,
ImmutableList.of("ns1.example.com", "ns2.example.com", "ns3.example.com"),
fakeClock.nowUtc()))
core/src/test/java/google/registry/testing/TestCacheExtension.java
+7 -2
View File
@@ -60,8 +60,13 @@ public class TestCacheExtension implements BeforeEachCallback, AfterEachCallback
return this;
}
public Builder withForeignKeyCache(Duration expiry) {
cacheHandlers.add(new TestCacheHandler(ForeignKeyUtils::setCacheForTest, expiry));
public Builder withForeignKeyRepoIdCache(Duration expiry) {
cacheHandlers.add(new TestCacheHandler(ForeignKeyUtils::setRepoIdCacheForTest, expiry));
return this;
}
public Builder withForeignKeyResourceCache(Duration expiry) {
cacheHandlers.add(new TestCacheHandler(ForeignKeyUtils::setResourceCacheForTest, expiry));
return this;
}
core/src/test/java/google/registry/tmch/NordnUploadActionTest.java
+16 -17
View File
@@ -19,9 +19,7 @@ import static com.google.common.net.HttpHeaders.CONTENT_TYPE;
import static com.google.common.net.HttpHeaders.LOCATION;
import static com.google.common.net.MediaType.FORM_DATA;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.model.ForeignKeyUtils.load;
import static google.registry.testing.DatabaseHelper.createTld;
import static google.registry.testing.DatabaseHelper.loadByKey;
import static google.registry.testing.DatabaseHelper.loadRegistrar;
import static google.registry.testing.DatabaseHelper.newDomain;
import static google.registry.testing.DatabaseHelper.persistResource;
@@ -40,10 +38,10 @@ import static org.mockito.Mockito.when;
import com.google.common.base.VerifyException;
import google.registry.batch.CloudTasksUtils;
import google.registry.model.ForeignKeyUtils;
import google.registry.model.domain.Domain;
import google.registry.model.domain.launch.LaunchNotice;
import google.registry.model.tld.Tld;
import google.registry.persistence.VKey;
import google.registry.persistence.transaction.JpaTestExtensions;
import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension;
import google.registry.request.RequestParameters;
@@ -72,19 +70,19 @@ class NordnUploadActionTest {
private static final String CLAIMS_CSV =
"""
1,2010-05-04T10:11:12.000Z,2
roid,domain-name,notice-id,registrar-id,registration-datetime,ack-datetime,application-datetime
6-TLD,claims-landrush2.tld,landrush2tcn,88888,2010-05-03T10:11:12.000Z,2010-05-03T08:11:12.000Z
8-TLD,claims-landrush1.tld,landrush1tcn,99999,2010-05-04T10:11:12.000Z,2010-05-04T09:11:12.000Z
""";
1,2010-05-04T10:11:12.000Z,2
roid,domain-name,notice-id,registrar-id,registration-datetime,ack-datetime,application-datetime
6-TLD,claims-landrush2.tld,landrush2tcn,88888,2010-05-03T10:11:12.000Z,2010-05-03T08:11:12.000Z
8-TLD,claims-landrush1.tld,landrush1tcn,99999,2010-05-04T10:11:12.000Z,2010-05-04T09:11:12.000Z
""";
private static final String SUNRISE_CSV =
"""
1,2010-05-04T10:11:12.000Z,2
roid,domain-name,SMD-id,registrar-id,registration-datetime,application-datetime
2-TLD,sunrise2.tld,new-smdid,88888,2010-05-01T10:11:12.000Z
4-TLD,sunrise1.tld,my-smdid,99999,2010-05-02T10:11:12.000Z
""";
1,2010-05-04T10:11:12.000Z,2
roid,domain-name,SMD-id,registrar-id,registration-datetime,application-datetime
2-TLD,sunrise2.tld,new-smdid,88888,2010-05-01T10:11:12.000Z
4-TLD,sunrise1.tld,my-smdid,99999,2010-05-02T10:11:12.000Z
""";
private static final String LOCATION_URL = "http://trololol";
@@ -142,12 +140,14 @@ class NordnUploadActionTest {
@Test
void testSuccess_nothingScheduled() {
persistResource(
loadByKey(load(Domain.class, "claims-landrush1.tld", clock.nowUtc()))
ForeignKeyUtils.loadResource(Domain.class, "claims-landrush1.tld", clock.nowUtc())
.get()
.asBuilder()
.setLordnPhase(LordnPhase.NONE)
.build());
persistResource(
loadByKey(load(Domain.class, "claims-landrush2.tld", clock.nowUtc()))
ForeignKeyUtils.loadResource(Domain.class, "claims-landrush2.tld", clock.nowUtc())
.get()
.asBuilder()
.setLordnPhase(LordnPhase.NONE)
.build());
@@ -233,8 +233,7 @@ class NordnUploadActionTest {
}
private void verifyColumnCleared(String domainName) {
VKey<Domain> domainKey = load(Domain.class, domainName, clock.nowUtc());
Domain domain = loadByKey(domainKey);
Domain domain = ForeignKeyUtils.loadResource(Domain.class, domainName, clock.nowUtc()).get();
assertThat(domain.getLordnPhase()).isEqualTo(LordnPhase.NONE);
}
core/src/test/java/google/registry/tools/EnqueuePollMessageCommandTest.java
+4 -3
View File
@@ -169,13 +169,14 @@ class EnqueuePollMessageCommandTest extends CommandTestCase<EnqueuePollMessageCo
@Test
void testNonexistentDomain() throws Exception {
IllegalArgumentException thrown =
RuntimeException thrown =
assertThrows(
IllegalArgumentException.class,
RuntimeException.class,
() -> runCommandForced("--domain=example2.tld", "--message=This domain needs help"));
assertThat(thrown)
.hasCauseThat()
.hasMessageThat()
.isEqualTo("Domain example2.tld doesn't exist or isn't active");
.isEqualTo("The domain with given ID (example2.tld) doesn't exist.");
}
@Test
core/src/test/java/google/registry/tools/RenewDomainCommandTest.java
+12 -7
View File
@@ -24,6 +24,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
import com.beust.jcommander.ParameterException;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import google.registry.flows.ResourceFlowUtils;
import google.registry.model.domain.Domain;
import google.registry.model.registrar.Registrar;
import google.registry.testing.DatabaseHelper;
@@ -174,19 +175,23 @@ public class RenewDomainCommandTest extends EppToolCommandTestCase<RenewDomainCo
@Test
void testFailure_domainDoesntExist() {
IllegalArgumentException e =
assertThrows(IllegalArgumentException.class, () -> runCommandForced("nonexistent.tld"));
assertThat(e)
assertThat(
assertThrows(
ResourceFlowUtils.ResourceDoesNotExistException.class,
() -> runCommandForced("nonexistent.tld")))
.hasMessageThat()
.isEqualTo("Domain 'nonexistent.tld' does not exist or is deleted");
.isEqualTo("The domain with given ID (nonexistent.tld) doesn't exist.");
}
@Test
void testFailure_domainIsDeleted() {
persistDeletedDomain("deleted.tld", DateTime.parse("2012-10-05T05:05:05Z"));
IllegalArgumentException e =
assertThrows(IllegalArgumentException.class, () -> runCommandForced("deleted.tld"));
assertThat(e).hasMessageThat().isEqualTo("Domain 'deleted.tld' does not exist or is deleted");
assertThat(
assertThrows(
ResourceFlowUtils.ResourceDoesNotExistException.class,
() -> runCommandForced("deleted.tld")))
.hasMessageThat()
.isEqualTo("The domain with given ID (deleted.tld) doesn't exist.");
}
@Test