From 56b61ad5a2089e44c634081212b58d26f9a7562b Mon Sep 17 00:00:00 2001 From: mcilwain Date: Wed, 19 Dec 2018 08:52:23 -0800 Subject: [PATCH] Invalidate premium list cache on update This will only affect the tools service, the primary use case being (1) I go to create a domain through nomulus tool, realize it's premium, (2) update the premium list to not include that domain, (3) kill the tools service instance to wipe out the cached premium value, then (4) create the domain at standard. This commit eliminates step 3. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=226180160 --- .../model/registry/label/PremiumList.java | 26 ++++++++++++------- .../registry/label/PremiumListUtils.java | 6 +++++ .../registry/label/PremiumListUtilsTest.java | 19 +++++++++++--- 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/java/google/registry/model/registry/label/PremiumList.java b/java/google/registry/model/registry/label/PremiumList.java index c4eb49d52..40b9b62b0 100644 --- a/java/google/registry/model/registry/label/PremiumList.java +++ b/java/google/registry/model/registry/label/PremiumList.java @@ -132,16 +132,22 @@ public final class PremiumList extends BaseDomainLabelListThis is cached for a shorter duration because we need to periodically reload this entity to * check if a new revision has been published, and if so, then use that. */ - static final LoadingCache cachePremiumLists = - CacheBuilder.newBuilder() - .expireAfterWrite(getDomainLabelListCacheDuration().getMillis(), MILLISECONDS) - .build( - new CacheLoader() { - @Override - public PremiumList load(final String name) { - return ofy().doTransactionless(() -> loadPremiumList(name)); - } - }); + @NonFinalForTesting + static LoadingCache cachePremiumLists = + createCachePremiumLists(getDomainLabelListCacheDuration()); + + @VisibleForTesting + static LoadingCache createCachePremiumLists(Duration cachePersistDuration) { + return CacheBuilder.newBuilder() + .expireAfterWrite(cachePersistDuration.getMillis(), MILLISECONDS) + .build( + new CacheLoader() { + @Override + public PremiumList load(final String name) { + return ofy().doTransactionless(() -> loadPremiumList(name)); + } + }); + } private static PremiumList loadPremiumList(String name) { return ofy().load().type(PremiumList.class).parent(getCrossTldKey()).id(name).now(); diff --git a/java/google/registry/model/registry/label/PremiumListUtils.java b/java/google/registry/model/registry/label/PremiumListUtils.java index e9c1515d7..d8df27851 100644 --- a/java/google/registry/model/registry/label/PremiumListUtils.java +++ b/java/google/registry/model/registry/label/PremiumListUtils.java @@ -174,6 +174,12 @@ public final class PremiumListUtils { ofy().save().entities(newList, newRevision); return newList; }); + + // Invalidate the cache on this premium list so the change will take effect instantly. This only + // clears the cache on the same instance that the update was run on, which will typically be the + // only tools instance. + PremiumList.cachePremiumLists.invalidate(premiumList.getName()); + // TODO(b/79888775): Enqueue the oldPremiumList for deletion after at least // RegistryConfig.getDomainLabelListCacheDuration() has elapsed. return updated; diff --git a/javatests/google/registry/model/registry/label/PremiumListUtilsTest.java b/javatests/google/registry/model/registry/label/PremiumListUtilsTest.java index 0feb5e9b4..59c2fc9aa 100644 --- a/javatests/google/registry/model/registry/label/PremiumListUtilsTest.java +++ b/javatests/google/registry/model/registry/label/PremiumListUtilsTest.java @@ -26,6 +26,7 @@ import static google.registry.model.registry.label.DomainLabelMetrics.PremiumLis import static google.registry.model.registry.label.DomainLabelMetrics.PremiumListCheckOutcome.UNCACHED_POSITIVE; import static google.registry.model.registry.label.DomainLabelMetrics.premiumListChecks; import static google.registry.model.registry.label.DomainLabelMetrics.premiumListProcessingTime; +import static google.registry.model.registry.label.PremiumList.createCachePremiumLists; import static google.registry.model.registry.label.PremiumListUtils.deletePremiumList; import static google.registry.model.registry.label.PremiumListUtils.doesPremiumListExist; import static google.registry.model.registry.label.PremiumListUtils.getPremiumPrice; @@ -35,7 +36,7 @@ import static google.registry.testing.DatastoreHelper.loadPremiumListEntries; import static google.registry.testing.DatastoreHelper.persistPremiumList; import static google.registry.testing.DatastoreHelper.persistResource; import static google.registry.testing.JUnitBackports.assertThrows; -import static org.joda.time.Duration.standardMinutes; +import static org.joda.time.Duration.standardDays; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -63,9 +64,11 @@ public class PremiumListUtilsTest { @Before public void before() { - // createTld() overwrites the premium list, so call it first. + // Set long persist times on caches so they can be tested (cache times default to 0 in tests). PremiumList.cachePremiumListEntries = - PremiumList.createCachePremiumListEntries(standardMinutes(1)); + PremiumList.createCachePremiumListEntries(standardDays(1)); + PremiumList.cachePremiumLists = createCachePremiumLists(standardDays(1)); + // createTld() overwrites the premium list, so call it first. createTld("tld"); PremiumList pl = persistPremiumList( @@ -309,6 +312,16 @@ public class PremiumListUtilsTest { assertThat(entriesReloaded.get("test").parent).isEqualTo(resaved.getRevisionKey()); } + @Test + public void test_savePremiumListAndEntries_clearsCache() { + assertThat(PremiumList.cachePremiumLists.getIfPresent("tld")).isNull(); + PremiumList pl = PremiumList.getCached("tld").get(); + assertThat(PremiumList.cachePremiumLists.getIfPresent("tld")).isEqualTo(pl); + savePremiumListAndEntries( + new PremiumList.Builder().setName("tld").build(), ImmutableList.of("test,USD 1")); + assertThat(PremiumList.cachePremiumLists.getIfPresent("tld")).isNull(); + } + @Test public void testDelete() { persistPremiumList("gtld1", "trombone,USD 10");