diff --git a/core/src/main/java/google/registry/tools/MutatingCommand.java b/core/src/main/java/google/registry/tools/MutatingCommand.java index 02bd6488a..3eba52689 100644 --- a/core/src/main/java/google/registry/tools/MutatingCommand.java +++ b/core/src/main/java/google/registry/tools/MutatingCommand.java @@ -55,7 +55,9 @@ public abstract class MutatingCommand extends ConfirmingCommand implements Comma /** The possible types of mutation that can be performed on an entity. */ public enum ChangeType { - CREATE, DELETE, UPDATE; + CREATE, + DELETE, + UPDATE; /** Return the ChangeType corresponding to the given combination of version existences. */ public static ChangeType get(boolean hasOldVersion, boolean hasNewVersion) { @@ -78,7 +80,7 @@ public abstract class MutatingCommand extends ConfirmingCommand implements Comma /** The key that points to the entity being changed. */ final VKey key; - public EntityChange(ImmutableObject oldEntity, ImmutableObject newEntity) { + private EntityChange(ImmutableObject oldEntity, ImmutableObject newEntity) { type = ChangeType.get(oldEntity != null, newEntity != null); checkArgument( type != ChangeType.UPDATE || Key.create(oldEntity).equals(Key.create(newEntity)), @@ -96,6 +98,34 @@ public abstract class MutatingCommand extends ConfirmingCommand implements Comma : VKey.createOfy(entity.getClass(), Key.create(entity)); } + /** + * EntityChange constructor that supports Vkey override. A Vkey is a key of an entity. This is a + * workaround to handle cases when a SqlEntity instance does not have a primary key before being + * persisted. + */ + private EntityChange(ImmutableObject oldEntity, ImmutableObject newEntity, VKey vkey) { + type = ChangeType.get(oldEntity != null, newEntity != null); + Key oldKey = Key.create(oldEntity), newKey = Key.create(newEntity); + if (type == ChangeType.UPDATE) { + checkArgument( + oldKey.equals(newKey), "Both entity versions in an update must have the same Key."); + checkArgument( + oldKey.equals(vkey.getOfyKey()), + "The Key of the entity must be the same as the OfyKey of the vkey"); + } else if (type == ChangeType.CREATE) { + checkArgument( + newKey.equals(vkey.getOfyKey()), + "Both entity versions in an update must have the same Key."); + } else if (type == ChangeType.DELETE) { + checkArgument( + oldKey.equals(vkey.getOfyKey()), + "The Key of the entity must be the same as the OfyKey of the vkey"); + } + this.oldEntity = oldEntity; + this.newEntity = newEntity; + key = vkey; + } + /** Returns a human-readable ID string for the entity being changed. */ public String getEntityId() { return String.format( @@ -110,8 +140,9 @@ public abstract class MutatingCommand extends ConfirmingCommand implements Comma public String toString() { String changeText; if (type == ChangeType.UPDATE) { - String diffText = prettyPrintEntityDeepDiff( - oldEntity.toDiffableFieldMap(), newEntity.toDiffableFieldMap()); + String diffText = + prettyPrintEntityDeepDiff( + oldEntity.toDiffableFieldMap(), newEntity.toDiffableFieldMap()); changeText = Optional.ofNullable(emptyToNull(diffText)).orElse("[no changes]\n"); } else { changeText = MoreObjects.firstNonNull(oldEntity, newEntity) + "\n"; @@ -205,8 +236,8 @@ public abstract class MutatingCommand extends ConfirmingCommand implements Comma } /** - * Subclasses can call this to stage a mutation to an entity that will be applied by execute(). - * Note that both objects passed must correspond to versions of the same entity with the same key. + * Stages an entity change that will be applied by execute(). Both ImmutableObject instances must + * be some version of the same entity with the same key. * * @param oldEntity the existing version of the entity, or null to create a new entity * @param newEntity the new version of the entity to save, or null to delete the entity @@ -222,6 +253,25 @@ public abstract class MutatingCommand extends ConfirmingCommand implements Comma lastAddedKey = change.key; } + /** + * Stages an entity change which will be applied by execute(), with the support of Vkey override. + * It supports cases of SqlEntity instances that do not have primary keys before being persisted. + * + * @param oldEntity the existing version of the entity, or null to create a new entity + * @param newEntity the new version of the entity to save, or null to delete the entity + * @param vkey the key of the entity + */ + protected void stageEntityChange( + @Nullable ImmutableObject oldEntity, @Nullable ImmutableObject newEntity, VKey vkey) { + EntityChange change = new EntityChange(oldEntity, newEntity, vkey); + checkArgument( + !changedEntitiesMap.containsKey(change.key), + "Cannot apply multiple changes for the same entity: %s", + change.getEntityId()); + changedEntitiesMap.put(change.key, change); + lastAddedKey = change.key; + } + /** * Subclasses can call this to write out all previously requested entity changes since the last * transaction flush in a transaction. diff --git a/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java b/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java index 01e7cd5af..f99a041b6 100644 --- a/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java +++ b/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java @@ -14,17 +14,17 @@ package google.registry.tools; -import static com.google.common.base.Preconditions.checkArgument; import static google.registry.util.ListNamingUtils.convertFilePathToName; import static java.nio.charset.StandardCharsets.UTF_8; import com.beust.jcommander.Parameters; import com.google.common.base.Strings; +import com.googlecode.objectify.Key; import google.registry.model.registry.label.ReservedList; +import google.registry.persistence.VKey; import google.registry.util.SystemClock; import java.nio.file.Files; import java.util.List; -import java.util.Optional; import org.joda.time.DateTime; /** Command to safely update {@link ReservedList} on Datastore. */ @@ -34,20 +34,38 @@ final class UpdateReservedListCommand extends CreateOrUpdateReservedListCommand @Override protected void init() throws Exception { name = Strings.isNullOrEmpty(name) ? convertFilePathToName(input) : name; - Optional existing = ReservedList.get(name); - checkArgument( - existing.isPresent(), "Could not update reserved list %s because it doesn't exist.", name); + ReservedList existingReservedList = + ReservedList.get(name) + .orElseThrow( + () -> + new IllegalArgumentException( + String.format( + "Could not update reserved list %s because it doesn't exist.", name))); boolean shouldPublish = - this.shouldPublish == null ? existing.get().getShouldPublish() : this.shouldPublish; + this.shouldPublish == null ? existingReservedList.getShouldPublish() : this.shouldPublish; List allLines = Files.readAllLines(input, UTF_8); DateTime now = new SystemClock().nowUtc(); ReservedList.Builder updated = - existing - .get() + existingReservedList .asBuilder() .setReservedListMapFromLines(allLines) .setLastUpdateTime(now) .setShouldPublish(shouldPublish); reservedList = updated.build(); + // only call stageEntityChange if there are changes in entries + + if (!existingReservedList + .getReservedListEntries() + .equals(reservedList.getReservedListEntries())) { + // calls the stageEntityChange method that takes old entity, new entity and a new vkey; + // a vkey has to be created here explicitly for ReservedList instances. + // ReservedList is a sqlEntity; it triggers the static method Vkey.create(Key ofyCall), + // which invokes a static ReservedList.createVkey(Key ofyKey) method that does not exist. + // the sql primary key field (revisionId) is only set when it's being persisted; + stageEntityChange( + existingReservedList, + reservedList, + VKey.createOfy(ReservedList.class, Key.create(existingReservedList))); + } } } diff --git a/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java b/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java index c661851ab..0dff08ff9 100644 --- a/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java @@ -18,14 +18,19 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; import static google.registry.model.registry.label.ReservationType.FULLY_BLOCKED; import static google.registry.testing.DatabaseHelper.persistReservedList; +import static google.registry.testing.TestDataHelper.loadFile; import static google.registry.util.DateTimeUtils.START_OF_TIME; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.io.Files; import google.registry.model.registry.label.ReservedList; import google.registry.model.registry.label.ReservedList.ReservedListEntry; import google.registry.model.registry.label.ReservedListSqlDao; +import java.io.File; +import java.nio.file.Paths; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -135,4 +140,38 @@ class UpdateReservedListCommandTest verifyXnq9jyb4cInDatastore(); assertThat(ReservedListSqlDao.checkExists("xn--q9jyb4c_common-reserved")).isTrue(); } + + @Test + void testSuccess_noChanges() throws Exception { + File reservedTermsFile = tmpDir.resolve("xn--q9jyb4c_common-reserved.txt").toFile(); + // after running runCommandForced, the file now contains "helicopter,FULLY_BLOCKED" which is + // populated in the @BeforeEach method of this class and the rest of terms from + // example_reserved_terms.csv, which are populated in the @BeforeEach of + // CreateOrUpdateReservedListCommandTestCases.java. + runCommandForced("--name=xn--q9jyb4c_common-reserved", "--input=" + reservedTermsPath); + + // set up to write content already in file + String reservedTermsCsv = + loadFile(CreateOrUpdateReservedListCommandTestCase.class, "example_reserved_terms.csv"); + Files.asCharSink(reservedTermsFile, UTF_8).write(reservedTermsCsv); + reservedTermsPath = reservedTermsFile.getPath(); + // create a command instance and assign its input + UpdateReservedListCommand command = new UpdateReservedListCommand(); + command.input = Paths.get(reservedTermsPath); + // run again with terms from example_reserved_terms.csv + command.init(); + + assertThat(command.prompt()).isEqualTo("No entity changes to apply."); + } + + @Test + void testSuccess_withChanges() throws Exception { + // changes come from example_reserved_terms.csv, which are populated in @BeforeEach of + // CreateOrUpdateReservedListCommandTestCases.java + UpdateReservedListCommand command = new UpdateReservedListCommand(); + command.input = Paths.get(reservedTermsPath); + command.init(); + + assertThat(command.prompt()).contains("Update ReservedList@xn--q9jyb4c_common-reserved"); + } }