From 74f0a8dd7bae905ba2e58bdd2ea7ced1722cd82d Mon Sep 17 00:00:00 2001 From: sarahcaseybot Date: Tue, 9 Jul 2024 16:05:15 -0400 Subject: [PATCH] Add nomulus tool command for FeatureFlags (#2480) * Add registryTool commands for FeatureFlags * Fix merge conflicts * Add required parameters and inject mapper * Use optionals in cache to negative cahe missing objects * Fix spelling * Change back to bulk load in cache * Add FeatureName enum * Change variable name * Use FeatureName in main parameter --- .../registry/model/EntityYamlUtils.java | 28 +++ .../registry/model/common/FeatureFlag.java | 89 +++++--- .../java/google/registry/model/tld/Tld.java | 32 ++- .../tools/ConfigureFeatureFlagCommand.java | 55 +++++ ...eateOrUpdateBulkPricingPackageCommand.java | 4 +- .../registry/tools/GetFeatureFlagCommand.java | 53 +++++ .../tools/ListFeatureFlagsCommand.java | 43 ++++ .../google/registry/tools/RegistryTool.java | 3 + .../registry/tools/RegistryToolComponent.java | 4 + .../tools/params/TransitionListParameter.java | 9 + .../model/common/FeatureFlagTest.java | 95 ++++---- .../ConfigureFeatureFlagCommandTest.java | 208 ++++++++++++++++++ .../tools/GetFeatureFlagCommandTest.java | 130 +++++++++++ .../tools/ListFeatureFlagsCommandTest.java | 89 ++++++++ .../google/registry/tools/oneFlag.yaml | 4 + .../google/registry/tools/threeFlags.yaml | 15 ++ 16 files changed, 750 insertions(+), 111 deletions(-) create mode 100644 core/src/main/java/google/registry/tools/ConfigureFeatureFlagCommand.java create mode 100644 core/src/main/java/google/registry/tools/GetFeatureFlagCommand.java create mode 100644 core/src/main/java/google/registry/tools/ListFeatureFlagsCommand.java create mode 100644 core/src/test/java/google/registry/tools/ConfigureFeatureFlagCommandTest.java create mode 100644 core/src/test/java/google/registry/tools/GetFeatureFlagCommandTest.java create mode 100644 core/src/test/java/google/registry/tools/ListFeatureFlagsCommandTest.java create mode 100644 core/src/test/resources/google/registry/tools/oneFlag.yaml create mode 100644 core/src/test/resources/google/registry/tools/threeFlags.yaml diff --git a/core/src/main/java/google/registry/model/EntityYamlUtils.java b/core/src/main/java/google/registry/model/EntityYamlUtils.java index e5c69fb0f..42be563ee 100644 --- a/core/src/main/java/google/registry/model/EntityYamlUtils.java +++ b/core/src/main/java/google/registry/model/EntityYamlUtils.java @@ -31,6 +31,7 @@ import com.fasterxml.jackson.databind.ser.std.StdSerializer; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator.Feature; import com.google.common.collect.ImmutableSortedSet; +import google.registry.model.common.FeatureFlag.FeatureStatus; import google.registry.model.common.TimedTransitionProperty; import google.registry.model.domain.token.AllocationToken; import google.registry.model.tld.Tld.TldState; @@ -363,6 +364,33 @@ public class EntityYamlUtils { } } + /** A custom JSON deserializer for a {@link TimedTransitionProperty} of {@link FeatureStatus}. */ + public static class TimedTransitionPropertyFeatureStatusDeserializer + extends StdDeserializer> { + + public TimedTransitionPropertyFeatureStatusDeserializer() { + this(null); + } + + public TimedTransitionPropertyFeatureStatusDeserializer( + Class> t) { + super(t); + } + + @Override + public TimedTransitionProperty deserialize( + JsonParser jp, DeserializationContext context) throws IOException { + SortedMap valueMap = jp.readValueAs(SortedMap.class); + return TimedTransitionProperty.fromValueMap( + valueMap.keySet().stream() + .collect( + toImmutableSortedMap( + natural(), + DateTime::parse, + key -> FeatureStatus.valueOf(valueMap.get(key))))); + } + } + /** A custom JSON deserializer for a {@link CreateAutoTimestamp}. */ public static class CreateAutoTimestampDeserializer extends StdDeserializer { diff --git a/core/src/main/java/google/registry/model/common/FeatureFlag.java b/core/src/main/java/google/registry/model/common/FeatureFlag.java index 0e5bc7e6c..cf1992b5a 100644 --- a/core/src/main/java/google/registry/model/common/FeatureFlag.java +++ b/core/src/main/java/google/registry/model/common/FeatureFlag.java @@ -14,29 +14,34 @@ package google.registry.model.common; -import static com.google.api.client.util.Preconditions.checkState; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.config.RegistryConfig.getSingletonCacheRefreshDuration; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.github.benmanes.caffeine.cache.CacheLoader; import com.github.benmanes.caffeine.cache.LoadingCache; import com.google.common.base.Joiner; -import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; -import com.google.common.collect.Sets; +import com.google.common.collect.Maps; import google.registry.model.Buildable; import google.registry.model.CacheUtils; +import google.registry.model.EntityYamlUtils.TimedTransitionPropertyFeatureStatusDeserializer; import google.registry.model.ImmutableObject; import google.registry.persistence.VKey; import java.util.Map; +import java.util.Optional; import java.util.Set; import javax.persistence.Column; import javax.persistence.Entity; +import javax.persistence.EnumType; +import javax.persistence.Enumerated; import javax.persistence.Id; import org.joda.time.DateTime; @@ -54,60 +59,76 @@ public class FeatureFlag extends ImmutableObject implements Buildable { INACTIVE } + public enum FeatureName { + TEST_FEATURE, + MINIMUM_DATASET_CONTACTS_OPTIONAL, + MINIMUM_DATASET_CONTACTS_PROHIBITED + } + /** The name of the flag/feature. */ - @Id String featureName; + @Enumerated(EnumType.STRING) + @Id + FeatureName featureName; /** A map of times for each {@link FeatureStatus} the FeatureFlag should hold. */ @Column(nullable = false) + @JsonDeserialize(using = TimedTransitionPropertyFeatureStatusDeserializer.class) TimedTransitionProperty status = TimedTransitionProperty.withInitialValue(FeatureStatus.INACTIVE); - public static FeatureFlag get(String featureName) { - FeatureFlag maybeFeatureFlag = CACHE.get(featureName); - if (maybeFeatureFlag == null) { - throw new FeatureFlagNotFoundException(featureName); - } else { - return maybeFeatureFlag; - } + public static Optional getUncached(FeatureName featureName) { + return tm().transact(() -> tm().loadByKeyIfPresent(createVKey(featureName))); } - public static ImmutableSet get(Set featureNames) { - Map featureFlags = CACHE.getAll(featureNames); - ImmutableSet missingFlags = - Sets.difference(featureNames, featureFlags.keySet()).immutableCopy(); + public static ImmutableList getAllUncached() { + return tm().transact(() -> tm().loadAllOf(FeatureFlag.class)); + } + + public static FeatureFlag get(FeatureName featureName) { + Optional maybeFeatureFlag = CACHE.get(featureName); + return maybeFeatureFlag.orElseThrow(() -> new FeatureFlagNotFoundException(featureName)); + } + + public static ImmutableSet getAll(Set featureNames) { + Map> featureFlags = CACHE.getAll(featureNames); + ImmutableSet missingFlags = + featureFlags.entrySet().stream() + .filter(e -> e.getValue().isEmpty()) + .map(Map.Entry::getKey) + .collect(toImmutableSet()); if (missingFlags.isEmpty()) { - return featureFlags.values().stream().collect(toImmutableSet()); + return featureFlags.values().stream().map(Optional::get).collect(toImmutableSet()); } else { throw new FeatureFlagNotFoundException(missingFlags); } } /** A cache that loads the {@link FeatureFlag} for a given featureName. */ - private static final LoadingCache CACHE = + private static final LoadingCache> CACHE = CacheUtils.newCacheBuilder(getSingletonCacheRefreshDuration()) .build( new CacheLoader<>() { @Override - public FeatureFlag load(final String featureName) { - return tm().reTransact(() -> tm().loadByKeyIfPresent(createVKey(featureName))) - .orElse(null); + public Optional load(final FeatureName featureName) { + return tm().reTransact(() -> tm().loadByKeyIfPresent(createVKey(featureName))); } @Override - public Map loadAll( - Set featureFlagNames) { - ImmutableMap> keysMap = + public Map> loadAll( + Set featureFlagNames) { + ImmutableMap> keysMap = featureFlagNames.stream() .collect( toImmutableMap(featureName -> featureName, FeatureFlag::createVKey)); Map, FeatureFlag> entities = tm().reTransact(() -> tm().loadByKeysIfPresent(keysMap.values())); - return entities.values().stream() - .collect(toImmutableMap(flag -> flag.featureName, flag -> flag)); + return Maps.toMap( + featureFlagNames, + name -> Optional.ofNullable(entities.get(createVKey(name)))); } }); - public static VKey createVKey(String featureName) { + public static VKey createVKey(FeatureName featureName) { return VKey.create(FeatureFlag.class, featureName); } @@ -116,10 +137,11 @@ public class FeatureFlag extends ImmutableObject implements Buildable { return createVKey(featureName); } - public String getFeatureName() { + public FeatureName getFeatureName() { return featureName; } + @JsonProperty("status") public TimedTransitionProperty getStatusMap() { return status; } @@ -144,20 +166,17 @@ public class FeatureFlag extends ImmutableObject implements Buildable { @Override public FeatureFlag build() { - checkArgument( - !Strings.isNullOrEmpty(getInstance().featureName), - "Feature name must not be null or empty"); getInstance().status.checkValidity(); + checkArgument(getInstance().featureName != null, "FeatureName cannot be null"); return super.build(); } - public Builder setFeatureName(String featureName) { - checkState(getInstance().featureName == null, "Feature name can only be set once"); + public Builder setFeatureName(FeatureName featureName) { getInstance().featureName = featureName; return this; } - public Builder setStatus(ImmutableSortedMap statusMap) { + public Builder setStatusMap(ImmutableSortedMap statusMap) { getInstance().status = TimedTransitionProperty.fromValueMap(statusMap); return this; } @@ -166,11 +185,11 @@ public class FeatureFlag extends ImmutableObject implements Buildable { /** Exception to throw when no FeatureFlag entity is found for given FeatureName string(s). */ public static class FeatureFlagNotFoundException extends RuntimeException { - FeatureFlagNotFoundException(ImmutableSet featureNames) { + FeatureFlagNotFoundException(ImmutableSet featureNames) { super("No feature flag object(s) found for " + Joiner.on(", ").join(featureNames)); } - FeatureFlagNotFoundException(String featureName) { + public FeatureFlagNotFoundException(FeatureName featureName) { this(ImmutableSet.of(featureName)); } } diff --git a/core/src/main/java/google/registry/model/tld/Tld.java b/core/src/main/java/google/registry/model/tld/Tld.java index dbed42170..cd2676d4e 100644 --- a/core/src/main/java/google/registry/model/tld/Tld.java +++ b/core/src/main/java/google/registry/model/tld/Tld.java @@ -41,9 +41,9 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; +import com.google.common.collect.Maps; import com.google.common.collect.Ordering; import com.google.common.collect.Range; -import com.google.common.collect.Sets; import com.google.common.net.InternetDomainName; import google.registry.model.Buildable; import google.registry.model.CacheUtils; @@ -192,21 +192,19 @@ public class Tld extends ImmutableObject implements Buildable, UnsafeSerializabl /** Returns the TLD for a given TLD, throwing if none exists. */ public static Tld get(String tld) { - Tld maybeTld = CACHE.get(tld); - if (maybeTld == null) { - throw new TldNotFoundException(tld); - } else { - return maybeTld; - } + return CACHE.get(tld).orElseThrow(() -> new TldNotFoundException(tld)); } /** Returns the TLD entities for the given TLD strings, throwing if any don't exist. */ public static ImmutableSet get(Set tlds) { - Map registries = CACHE.getAll(tlds); + Map> registries = CACHE.getAll(tlds); ImmutableSet missingRegistries = - Sets.difference(tlds, registries.keySet()).immutableCopy(); + registries.entrySet().stream() + .filter(e -> e.getValue().isEmpty()) + .map(Map.Entry::getKey) + .collect(toImmutableSet()); if (missingRegistries.isEmpty()) { - return registries.values().stream().collect(toImmutableSet()); + return registries.values().stream().map(Optional::get).collect(toImmutableSet()); } else { throw new TldNotFoundException(missingRegistries); } @@ -224,24 +222,24 @@ public class Tld extends ImmutableObject implements Buildable, UnsafeSerializabl } /** A cache that loads the {@link Tld} for a given tld. */ - private static final LoadingCache CACHE = + private static final LoadingCache> CACHE = CacheUtils.newCacheBuilder(getSingletonCacheRefreshDuration()) .build( new CacheLoader<>() { @Override - public Tld load(final String tld) { - return tm().reTransact(() -> tm().loadByKeyIfPresent(createVKey(tld))) - .orElse(null); + public Optional load(final String tld) { + return tm().reTransact(() -> tm().loadByKeyIfPresent(createVKey(tld))); } @Override - public Map loadAll(Set tlds) { + public Map> loadAll( + Set tlds) { ImmutableMap> keysMap = tlds.stream().collect(toImmutableMap(tld -> tld, Tld::createVKey)); Map, Tld> entities = tm().reTransact(() -> tm().loadByKeysIfPresent(keysMap.values())); - return entities.values().stream() - .collect(toImmutableMap(tld -> tld.tldStr, tld -> tld)); + return Maps.toMap( + tlds, tld -> Optional.ofNullable(entities.get(createVKey(tld)))); } }); diff --git a/core/src/main/java/google/registry/tools/ConfigureFeatureFlagCommand.java b/core/src/main/java/google/registry/tools/ConfigureFeatureFlagCommand.java new file mode 100644 index 000000000..e0a98a5df --- /dev/null +++ b/core/src/main/java/google/registry/tools/ConfigureFeatureFlagCommand.java @@ -0,0 +1,55 @@ +// Copyright 2024 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.tools; + +import com.beust.jcommander.Parameter; +import com.beust.jcommander.Parameters; +import com.google.common.collect.ImmutableSortedMap; +import google.registry.model.common.FeatureFlag; +import google.registry.model.common.FeatureFlag.FeatureName; +import google.registry.model.common.FeatureFlag.FeatureStatus; +import google.registry.tools.params.TransitionListParameter.FeatureStatusTransitions; +import java.util.List; +import java.util.Optional; +import org.joda.time.DateTime; + +/** Command for creating and updating {@link FeatureFlag} objects. */ +@Parameters(separators = " =", commandDescription = "Create or update a feature flag.") +public class ConfigureFeatureFlagCommand extends MutatingCommand { + + @Parameter(description = "Feature flag name(s) to create or update", required = true) + private List mainParameters; + + @Parameter( + names = "--status_map", + converter = FeatureStatusTransitions.class, + validateWith = FeatureStatusTransitions.class, + description = + "Comma-delimited list of feature status transitions effective on specific dates, of the" + + " form