From c12774fd1ea72b832fb8c3470082ca7acb3d9200 Mon Sep 17 00:00:00 2001 From: Rachel Guan Date: Wed, 22 Dec 2021 15:22:33 -0500 Subject: [PATCH] Change resource key type from Key to String for ResaveEntityAction (#1475) * Change resource key type from key to string for ResaveEntityAction * Remove throws Exception related to VKey.create() --- .../google/registry/batch/BatchModule.java | 7 +--- .../registry/batch/ResaveEntityAction.java | 10 ++--- .../module/backend/BackendComponent.java | 2 + .../google/registry/persistence/VKey.java | 2 +- .../tools/GetResourceByKeyCommand.java | 2 +- .../registry/tools/ResaveEntitiesCommand.java | 2 +- .../batch/ResaveEntityActionTest.java | 16 +++---- .../google/registry/persistence/VKeyTest.java | 42 +++++++++---------- 8 files changed, 41 insertions(+), 42 deletions(-) diff --git a/core/src/main/java/google/registry/batch/BatchModule.java b/core/src/main/java/google/registry/batch/BatchModule.java index 8baadb1d6..8e12a4f9f 100644 --- a/core/src/main/java/google/registry/batch/BatchModule.java +++ b/core/src/main/java/google/registry/batch/BatchModule.java @@ -33,10 +33,8 @@ import static google.registry.request.RequestParameters.extractSetOfDatetimePara import com.google.appengine.api.taskqueue.Queue; import com.google.common.collect.ImmutableSet; -import com.googlecode.objectify.Key; import dagger.Module; import dagger.Provides; -import google.registry.model.ImmutableObject; import google.registry.request.Parameter; import java.util.Optional; import javax.inject.Named; @@ -79,9 +77,8 @@ public class BatchModule { @Provides @Parameter(PARAM_RESOURCE_KEY) - // TODO(b/207363014): figure out if this needs to be modified for vkey string replacement - static Key provideResourceKey(HttpServletRequest req) { - return Key.create(extractRequiredParameter(req, PARAM_RESOURCE_KEY)); + static String provideResourceKey(HttpServletRequest req) { + return extractRequiredParameter(req, PARAM_RESOURCE_KEY); } @Provides diff --git a/core/src/main/java/google/registry/batch/ResaveEntityAction.java b/core/src/main/java/google/registry/batch/ResaveEntityAction.java index dbd40a81d..480e5a5c1 100644 --- a/core/src/main/java/google/registry/batch/ResaveEntityAction.java +++ b/core/src/main/java/google/registry/batch/ResaveEntityAction.java @@ -22,7 +22,6 @@ import static google.registry.persistence.transaction.TransactionManagerFactory. import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.flogger.FluentLogger; -import com.googlecode.objectify.Key; import google.registry.model.EppResource; import google.registry.model.ImmutableObject; import google.registry.persistence.VKey; @@ -50,7 +49,7 @@ public class ResaveEntityAction implements Runnable { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private final Key resourceKey; + private final String resourceKey; private final DateTime requestedTime; private final ImmutableSortedSet resaveTimes; private final AsyncTaskEnqueuer asyncTaskEnqueuer; @@ -58,7 +57,7 @@ public class ResaveEntityAction implements Runnable { @Inject ResaveEntityAction( - @Parameter(PARAM_RESOURCE_KEY) Key resourceKey, + @Parameter(PARAM_RESOURCE_KEY) String resourceKey, @Parameter(PARAM_REQUESTED_TIME) DateTime requestedTime, @Parameter(PARAM_RESAVE_TIMES) ImmutableSet resaveTimes, AsyncTaskEnqueuer asyncTaskEnqueuer, @@ -76,15 +75,14 @@ public class ResaveEntityAction implements Runnable { "Re-saving entity %s which was enqueued at %s.", resourceKey, requestedTime); tm().transact( () -> { - // TODO(/207363014): figure out if this should modified for vkey string replacement - ImmutableObject entity = tm().loadByKey(VKey.from(resourceKey)); + ImmutableObject entity = tm().loadByKey(VKey.create(resourceKey)); tm().put( (entity instanceof EppResource) ? ((EppResource) entity).cloneProjectedAtTime(tm().getTransactionTime()) : entity); if (!resaveTimes.isEmpty()) { asyncTaskEnqueuer.enqueueAsyncResave( - VKey.from(resourceKey), requestedTime, resaveTimes); + VKey.create(resourceKey), requestedTime, resaveTimes); } }); response.setPayload("Entity re-saved."); diff --git a/core/src/main/java/google/registry/module/backend/BackendComponent.java b/core/src/main/java/google/registry/module/backend/BackendComponent.java index da80cf220..9b615f0cd 100644 --- a/core/src/main/java/google/registry/module/backend/BackendComponent.java +++ b/core/src/main/java/google/registry/module/backend/BackendComponent.java @@ -17,6 +17,7 @@ package google.registry.module.backend; import com.google.monitoring.metrics.MetricReporter; import dagger.Component; import dagger.Lazy; +import google.registry.batch.BatchModule; import google.registry.bigquery.BigqueryModule; import google.registry.config.CloudTasksUtilsModule; import google.registry.config.CredentialModule; @@ -55,6 +56,7 @@ import javax.inject.Singleton; modules = { AuthModule.class, BackendRequestComponentModule.class, + BatchModule.class, BigqueryModule.class, ConfigModule.class, CloudTasksUtilsModule.class, diff --git a/core/src/main/java/google/registry/persistence/VKey.java b/core/src/main/java/google/registry/persistence/VKey.java index 505914879..f992a6223 100644 --- a/core/src/main/java/google/registry/persistence/VKey.java +++ b/core/src/main/java/google/registry/persistence/VKey.java @@ -142,7 +142,7 @@ public class VKey extends ImmutableObject implements Serializable { * "@ofy:agR0ZXN0cjELEg9FbnRpdHlHcm91cFJvb3QiCWNyb3NzLXRsZAwLEgpUZXN0T2JqZWN0IgNmb28M", where sql * key and ofy key values are encoded in Base64. */ - public static VKey create(String keyString) throws Exception { + public static VKey create(String keyString) { if (!keyString.startsWith(CLASS_TYPE + KV_SEPARATOR)) { // to handle the existing ofy key string return fromWebsafeKey(keyString); diff --git a/core/src/main/java/google/registry/tools/GetResourceByKeyCommand.java b/core/src/main/java/google/registry/tools/GetResourceByKeyCommand.java index df729c6c7..8a83e63b9 100644 --- a/core/src/main/java/google/registry/tools/GetResourceByKeyCommand.java +++ b/core/src/main/java/google/registry/tools/GetResourceByKeyCommand.java @@ -40,7 +40,7 @@ final class GetResourceByKeyCommand implements CommandWithRemoteApi { boolean expand; @Override - public void run() throws Exception { + public void run() { for (String keyString : mainParameters) { System.out.println("\n"); VKey resourceKey = diff --git a/core/src/main/java/google/registry/tools/ResaveEntitiesCommand.java b/core/src/main/java/google/registry/tools/ResaveEntitiesCommand.java index f239b95b4..6fdc4e497 100644 --- a/core/src/main/java/google/registry/tools/ResaveEntitiesCommand.java +++ b/core/src/main/java/google/registry/tools/ResaveEntitiesCommand.java @@ -42,7 +42,7 @@ public final class ResaveEntitiesCommand extends MutatingCommand { List mainParameters; @Override - protected void init() throws Exception { + protected void init() { for (List batch : partition(mainParameters, BATCH_SIZE)) { for (String websafeKey : batch) { ImmutableObject entity = diff --git a/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java b/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java index 2e9a14161..b8b3831f4 100644 --- a/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java +++ b/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java @@ -33,8 +33,6 @@ import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; -import com.googlecode.objectify.Key; -import google.registry.model.ImmutableObject; import google.registry.model.domain.DomainBase; import google.registry.model.domain.GracePeriod; import google.registry.model.domain.rgp.GracePeriodStatus; @@ -84,9 +82,7 @@ public class ResaveEntityActionTest { } private void runAction( - Key resourceKey, - DateTime requestedTime, - ImmutableSortedSet resaveTimes) { + String resourceKey, DateTime requestedTime, ImmutableSortedSet resaveTimes) { ResaveEntityAction action = new ResaveEntityAction( resourceKey, requestedTime, resaveTimes, asyncTaskEnqueuer, response); @@ -110,7 +106,10 @@ public class ResaveEntityActionTest { DateTime.parse("2017-01-02T10:11:00Z")); clock.advanceOneMilli(); assertThat(domain.getCurrentSponsorRegistrarId()).isEqualTo("TheRegistrar"); - runAction(Key.create(domain), DateTime.parse("2016-02-06T10:00:01Z"), ImmutableSortedSet.of()); + runAction( + domain.createVKey().getOfyKey().getString(), + DateTime.parse("2016-02-06T10:00:01Z"), + ImmutableSortedSet.of()); DomainBase resavedDomain = loadByEntity(domain); assertThat(resavedDomain.getCurrentSponsorRegistrarId()).isEqualTo("NewRegistrar"); verify(response).setPayload("Entity re-saved."); @@ -137,7 +136,10 @@ public class ResaveEntityActionTest { DateTime requestedTime = clock.nowUtc(); assertThat(domain.getGracePeriods()).isNotEmpty(); - runAction(Key.create(domain), requestedTime, ImmutableSortedSet.of(requestedTime.plusDays(5))); + runAction( + domain.createVKey().getOfyKey().getString(), + requestedTime, + ImmutableSortedSet.of(requestedTime.plusDays(5))); DomainBase resavedDomain = loadByEntity(domain); assertThat(resavedDomain.getGracePeriods()).isEmpty(); diff --git a/core/src/test/java/google/registry/persistence/VKeyTest.java b/core/src/test/java/google/registry/persistence/VKeyTest.java index 8bda290f0..ea685b4ea 100644 --- a/core/src/test/java/google/registry/persistence/VKeyTest.java +++ b/core/src/test/java/google/registry/persistence/VKeyTest.java @@ -159,19 +159,19 @@ class VKeyTest { /** Test stringify() with vkey created via different ways. */ @Test - void testStringify_sqlOnlyVKey() throws Exception { + void testStringify_sqlOnlyVKey() { assertThat(VKey.createSql(TestObject.class, "foo").stringify()) .isEqualTo("kind:TestObject@sql:rO0ABXQAA2Zvbw"); } @Test - void testStringify_ofyOnlyVKey() throws Exception { + void testStringify_ofyOnlyVKey() { assertThat(VKey.createOfy(TestObject.class, Key.create(TestObject.class, "foo")).stringify()) .isEqualTo("kind:TestObject@ofy:agR0ZXN0chMLEgpUZXN0T2JqZWN0IgNmb28M"); } @Test - void testStringify_vkeyFromWebsafeKey() throws Exception { + void testStringify_vkeyFromWebsafeKey() { DomainBase domain = newDomainBase("example.com", "ROID-1", persistActiveContact("contact-1")); Key key = Key.create(domain); VKey vkey = VKey.fromWebsafeKey(key.getString()); @@ -183,7 +183,7 @@ class VKeyTest { } @Test - void testStringify_sqlAndOfyVKey() throws Exception { + void testStringify_sqlAndOfyVKey() { assertThat( VKey.create(TestObject.class, "foo", Key.create(TestObject.create("foo"))).stringify()) .isEqualTo( @@ -192,7 +192,7 @@ class VKeyTest { } @Test - void testStringify_asymmetricVKey() throws Exception { + void testStringify_asymmetricVKey() { assertThat( VKey.create(TestObject.class, "test", Key.create(TestObject.create("foo"))).stringify()) .isEqualTo( @@ -202,19 +202,19 @@ class VKeyTest { /** Test create() via different vkey string representations. */ @Test - void testCreate_stringifedVKey_sqlOnlyVKeyString() throws Exception { + void testCreate_stringifedVKey_sqlOnlyVKeyString() { assertThat(VKey.create("kind:TestObject@sql:rO0ABXQAA2Zvbw")) .isEqualTo(VKey.createSql(TestObject.class, "foo")); } @Test - void testCreate_stringifedVKey_ofyOnlyVKeyString() throws Exception { + void testCreate_stringifedVKey_ofyOnlyVKeyString() { assertThat(VKey.create("kind:TestObject@ofy:agR0ZXN0chMLEgpUZXN0T2JqZWN0IgNmb28M")) .isEqualTo(VKey.createOfy(TestObject.class, Key.create(TestObject.class, "foo"))); } @Test - void testCreate_stringifedVKey_asymmetricVKeyString() throws Exception { + void testCreate_stringifedVKey_asymmetricVKeyString() { assertThat( VKey.create( "kind:TestObject@sql:rO0ABXQABHRlc3Q@ofy:agR0ZXN0cjELEg9Fb" @@ -223,7 +223,7 @@ class VKeyTest { } @Test - void testCreate_stringifedVKey_sqlAndOfyVKeyString() throws Exception { + void testCreate_stringifedVKey_sqlAndOfyVKeyString() { assertThat( VKey.create( "kind:TestObject@sql:rO0ABXQAA2Zvbw@ofy:agR0ZXN0cjELEg9Fbn" @@ -232,7 +232,7 @@ class VKeyTest { } @Test - void testCreate_stringifyVkey_fromWebsafeKey() throws Exception { + void testCreate_stringifyVkey_fromWebsafeKey() { assertThat( VKey.create( "kind:DomainBase@sql:rO0ABXQABlJPSUQtMQ" @@ -245,13 +245,13 @@ class VKeyTest { } @Test - void testCreate_stringifedVKey_websafeKey() throws Exception { + void testCreate_stringifedVKey_websafeKey() { assertThat(VKey.create("agR0ZXN0chYLEgpEb21haW5CYXNlIgZST0lELTEM")) .isEqualTo(VKey.fromWebsafeKey("agR0ZXN0chYLEgpEb21haW5CYXNlIgZST0lELTEM")); } @Test - void testCreate_invalidStringifiedVKey_failure() throws Exception { + void testCreate_invalidStringifiedVKey_failure() { IllegalArgumentException thrown = assertThrows( IllegalArgumentException.class, () -> VKey.create("kind:TestObject@sq:l@ofya:bc")); @@ -261,7 +261,7 @@ class VKeyTest { } @Test - void testCreate_invalidOfyKeyString_failure() throws Exception { + void testCreate_invalidOfyKeyString_failure() { IllegalArgumentException thrown = assertThrows(IllegalArgumentException.class, () -> VKey.create("invalid")); assertThat(thrown).hasMessageThat().contains("Could not parse Reference"); @@ -269,21 +269,21 @@ class VKeyTest { /** Test stringify() then create() flow. */ @Test - void testStringifyThenCreate_sqlOnlyVKey_testObject_stringKey_success() throws Exception { + void testStringifyThenCreate_sqlOnlyVKey_testObject_stringKey_success() { VKey vkey = VKey.createSql(TestObject.class, "foo"); VKey newVkey = VKey.create(vkey.stringify()); assertThat(newVkey).isEqualTo(vkey); } @Test - void testStringifyThenCreate_sqlOnlyVKey_testObject_longKey_success() throws Exception { + void testStringifyThenCreate_sqlOnlyVKey_testObject_longKey_success() { VKey vkey = VKey.createSql(TestObject.class, (long) 12345); VKey newVkey = VKey.create(vkey.stringify()); assertThat(newVkey).isEqualTo(vkey); } @Test - void testCreate_createFromExistingOfyKey_success() throws Exception { + void testCreate_createFromExistingOfyKey_success() { String keyString = Key.create(newDomainBase("example.com", "ROID-1", persistActiveContact("contact-1"))) .getString(); @@ -291,34 +291,34 @@ class VKeyTest { } @Test - void testStringifyThenCreate_ofyOnlyVKey_testObject_success() throws Exception { + void testStringifyThenCreate_ofyOnlyVKey_testObject_success() { VKey vkey = VKey.createOfy(TestObject.class, Key.create(TestObject.class, "tmpKey")); assertThat(VKey.create(vkey.stringify())).isEqualTo(vkey); } @Test - void testStringifyThenCreate_ofyOnlyVKey_testObject_websafeString_success() throws Exception { + void testStringifyThenCreate_ofyOnlyVKey_testObject_websafeString_success() { VKey vkey = VKey.fromWebsafeKey(Key.create(TestObject.create("foo")).getString()); assertThat(VKey.create(vkey.stringify())).isEqualTo(vkey); } @Test - void testStringifyThenCreate_sqlAndOfyVKey_success() throws Exception { + void testStringifyThenCreate_sqlAndOfyVKey_success() { VKey vkey = VKey.create(TestObject.class, "foo", Key.create(TestObject.create("foo"))); assertThat(VKey.create(vkey.stringify())).isEqualTo(vkey); } @Test - void testStringifyThenCreate_asymmetricVKey_success() throws Exception { + void testStringifyThenCreate_asymmetricVKey_success() { VKey vkey = VKey.create(TestObject.class, "sqlKey", Key.create(TestObject.create("foo"))); assertThat(VKey.create(vkey.stringify())).isEqualTo(vkey); } @Test - void testStringifyThenCreate_symmetricVKey_success() throws Exception { + void testStringifyThenCreate_symmetricVKey_success() { VKey vkey = TestObject.create("foo").key(); assertThat(VKey.create(vkey.stringify())).isEqualTo(vkey); }