zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-05-14 00:17:20 +02:00
Code Issues Projects Releases Packages Wiki Activity

Don't delete the old premium list yet after updating to a new one

Browse source 
The issue is that the premium list cache is configured to persist for 60
minutes. So after updating the list, checks/creates for up to the next 60
minutes could still be referring to the old list. That's fine and dandy, unless
you delete the old premium list immediately (*bad*), which makes all domains
appear to now be non-premium for as long as the cache lasts. The reason deleting
the premium list entries makes names appear as non-premium is that a load-by-key
existence check with the domain label itself is used to determine if a name is
premium.

I also removed a misleading cache update statement, which doesn't do what it
appears to be doing (it appears to fix this issue) because cache is
instance-level, and so even if the premium list were updated from the frontend
instance only one of 100 instances would have its cache updated. But it's
updated from the tools service anyway, so it's guaranteed to not be a shared
cache with any instance serving EPP traffic.

On a sidenote, I introduced this bug on 2014-10-27 in [] The domain
label list refactor was my Noogler project.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=197033604
This commit is contained in:
mcilwain 2018-05-17 12:30:52 -07:00 • committed by jianglai
parent 0fb845e81a
commit b8d7d9da29
2 changed files with 3 additions and 18 deletions
Download patch file Download diff file Expand all files Collapse all files

6
java/google/registry/model/registry/label/PremiumListUtils.java
View file

@ -180,10 +180,8 @@ public final class PremiumListUtils {
ofy().save().entities(newList, newRevision); ofy().save().entities(newList, newRevision);
return newList; return newList;
}); });
// Update the cache. // TODO(b/79888775): Enqueue the oldPremiumList for deletion after at least
cachePremiumLists.put(premiumList.getName(), updated); // RegistryConfig.getDomainLabelListCacheDuration() has elapsed.
// Delete the entities under the old PremiumList.
oldPremiumList.ifPresent(PremiumListUtils::deleteRevisionAndEntriesOfPremiumList);
return updated; return updated;
} }

15
javatests/google/registry/model/registry/label/PremiumListUtilsTest.java
View file

@ -244,20 +244,7 @@ public class PremiumListUtilsTest {
assertThat(getPremiumPrice("genius", registry)).hasValue(Money.parse("USD 10")); assertThat(getPremiumPrice("genius", registry)).hasValue(Money.parse("USD 10"));
assertThat(getPremiumPrice("savant", registry)).hasValue(Money.parse("USD 90")); assertThat(getPremiumPrice("savant", registry)).hasValue(Money.parse("USD 90"));
assertThat(getPremiumPrice("dolt", registry)).isEmpty(); assertThat(getPremiumPrice("dolt", registry)).isEmpty();
assertThat(ofy() // TODO(b/79888775): Assert that the old premium list is enqueued for later deletion.
.load()
.type(PremiumListEntry.class)
.parent(pl.getRevisionKey())
.id("dolt")
.now())
.isNull();
assertThat(ofy()
.load()
.type(PremiumListEntry.class)
.parent(pl2.getRevisionKey())
.id("dolt")
.now())
.isNull();
assertThat(premiumListChecks) assertThat(premiumListChecks)
.hasValueForLabels(4, "tld", "tld", UNCACHED_POSITIVE.toString()) .hasValueForLabels(4, "tld", "tld", UNCACHED_POSITIVE.toString())
.and() .and()