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()
This commit is contained in:
Rachel Guan 2021-12-22 15:22:33 -05:00 committed by GitHub
parent a440fbe9b0
commit c12774fd1e
8 changed files with 41 additions and 42 deletions

View file

@ -33,10 +33,8 @@ import static google.registry.request.RequestParameters.extractSetOfDatetimePara
import com.google.appengine.api.taskqueue.Queue; import com.google.appengine.api.taskqueue.Queue;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.googlecode.objectify.Key;
import dagger.Module; import dagger.Module;
import dagger.Provides; import dagger.Provides;
import google.registry.model.ImmutableObject;
import google.registry.request.Parameter; import google.registry.request.Parameter;
import java.util.Optional; import java.util.Optional;
import javax.inject.Named; import javax.inject.Named;
@ -79,9 +77,8 @@ public class BatchModule {
@Provides @Provides
@Parameter(PARAM_RESOURCE_KEY) @Parameter(PARAM_RESOURCE_KEY)
// TODO(b/207363014): figure out if this needs to be modified for vkey string replacement static String provideResourceKey(HttpServletRequest req) {
static Key<ImmutableObject> provideResourceKey(HttpServletRequest req) { return extractRequiredParameter(req, PARAM_RESOURCE_KEY);
return Key.create(extractRequiredParameter(req, PARAM_RESOURCE_KEY));
} }
@Provides @Provides

View file

@ -22,7 +22,6 @@ import static google.registry.persistence.transaction.TransactionManagerFactory.
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.ImmutableSortedSet;
import com.google.common.flogger.FluentLogger; import com.google.common.flogger.FluentLogger;
import com.googlecode.objectify.Key;
import google.registry.model.EppResource; import google.registry.model.EppResource;
import google.registry.model.ImmutableObject; import google.registry.model.ImmutableObject;
import google.registry.persistence.VKey; import google.registry.persistence.VKey;
@ -50,7 +49,7 @@ public class ResaveEntityAction implements Runnable {
private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final Key<ImmutableObject> resourceKey; private final String resourceKey;
private final DateTime requestedTime; private final DateTime requestedTime;
private final ImmutableSortedSet<DateTime> resaveTimes; private final ImmutableSortedSet<DateTime> resaveTimes;
private final AsyncTaskEnqueuer asyncTaskEnqueuer; private final AsyncTaskEnqueuer asyncTaskEnqueuer;
@ -58,7 +57,7 @@ public class ResaveEntityAction implements Runnable {
@Inject @Inject
ResaveEntityAction( ResaveEntityAction(
@Parameter(PARAM_RESOURCE_KEY) Key<ImmutableObject> resourceKey, @Parameter(PARAM_RESOURCE_KEY) String resourceKey,
@Parameter(PARAM_REQUESTED_TIME) DateTime requestedTime, @Parameter(PARAM_REQUESTED_TIME) DateTime requestedTime,
@Parameter(PARAM_RESAVE_TIMES) ImmutableSet<DateTime> resaveTimes, @Parameter(PARAM_RESAVE_TIMES) ImmutableSet<DateTime> resaveTimes,
AsyncTaskEnqueuer asyncTaskEnqueuer, AsyncTaskEnqueuer asyncTaskEnqueuer,
@ -76,15 +75,14 @@ public class ResaveEntityAction implements Runnable {
"Re-saving entity %s which was enqueued at %s.", resourceKey, requestedTime); "Re-saving entity %s which was enqueued at %s.", resourceKey, requestedTime);
tm().transact( tm().transact(
() -> { () -> {
// TODO(/207363014): figure out if this should modified for vkey string replacement ImmutableObject entity = tm().loadByKey(VKey.create(resourceKey));
ImmutableObject entity = tm().loadByKey(VKey.from(resourceKey));
tm().put( tm().put(
(entity instanceof EppResource) (entity instanceof EppResource)
? ((EppResource) entity).cloneProjectedAtTime(tm().getTransactionTime()) ? ((EppResource) entity).cloneProjectedAtTime(tm().getTransactionTime())
: entity); : entity);
if (!resaveTimes.isEmpty()) { if (!resaveTimes.isEmpty()) {
asyncTaskEnqueuer.enqueueAsyncResave( asyncTaskEnqueuer.enqueueAsyncResave(
VKey.from(resourceKey), requestedTime, resaveTimes); VKey.create(resourceKey), requestedTime, resaveTimes);
} }
}); });
response.setPayload("Entity re-saved."); response.setPayload("Entity re-saved.");

View file

@ -17,6 +17,7 @@ package google.registry.module.backend;
import com.google.monitoring.metrics.MetricReporter; import com.google.monitoring.metrics.MetricReporter;
import dagger.Component; import dagger.Component;
import dagger.Lazy; import dagger.Lazy;
import google.registry.batch.BatchModule;
import google.registry.bigquery.BigqueryModule; import google.registry.bigquery.BigqueryModule;
import google.registry.config.CloudTasksUtilsModule; import google.registry.config.CloudTasksUtilsModule;
import google.registry.config.CredentialModule; import google.registry.config.CredentialModule;
@ -55,6 +56,7 @@ import javax.inject.Singleton;
modules = { modules = {
AuthModule.class, AuthModule.class,
BackendRequestComponentModule.class, BackendRequestComponentModule.class,
BatchModule.class,
BigqueryModule.class, BigqueryModule.class,
ConfigModule.class, ConfigModule.class,
CloudTasksUtilsModule.class, CloudTasksUtilsModule.class,

View file

@ -142,7 +142,7 @@ public class VKey<T> extends ImmutableObject implements Serializable {
* "@ofy:agR0ZXN0cjELEg9FbnRpdHlHcm91cFJvb3QiCWNyb3NzLXRsZAwLEgpUZXN0T2JqZWN0IgNmb28M", where sql * "@ofy:agR0ZXN0cjELEg9FbnRpdHlHcm91cFJvb3QiCWNyb3NzLXRsZAwLEgpUZXN0T2JqZWN0IgNmb28M", where sql
* key and ofy key values are encoded in Base64. * key and ofy key values are encoded in Base64.
*/ */
public static <T> VKey<T> create(String keyString) throws Exception { public static <T> VKey<T> create(String keyString) {
if (!keyString.startsWith(CLASS_TYPE + KV_SEPARATOR)) { if (!keyString.startsWith(CLASS_TYPE + KV_SEPARATOR)) {
// to handle the existing ofy key string // to handle the existing ofy key string
return fromWebsafeKey(keyString); return fromWebsafeKey(keyString);

View file

@ -40,7 +40,7 @@ final class GetResourceByKeyCommand implements CommandWithRemoteApi {
boolean expand; boolean expand;
@Override @Override
public void run() throws Exception { public void run() {
for (String keyString : mainParameters) { for (String keyString : mainParameters) {
System.out.println("\n"); System.out.println("\n");
VKey<EppResource> resourceKey = VKey<EppResource> resourceKey =

View file

@ -42,7 +42,7 @@ public final class ResaveEntitiesCommand extends MutatingCommand {
List<String> mainParameters; List<String> mainParameters;
@Override @Override
protected void init() throws Exception { protected void init() {
for (List<String> batch : partition(mainParameters, BATCH_SIZE)) { for (List<String> batch : partition(mainParameters, BATCH_SIZE)) {
for (String websafeKey : batch) { for (String websafeKey : batch) {
ImmutableObject entity = ImmutableObject entity =

View file

@ -33,8 +33,6 @@ import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet; 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.DomainBase;
import google.registry.model.domain.GracePeriod; import google.registry.model.domain.GracePeriod;
import google.registry.model.domain.rgp.GracePeriodStatus; import google.registry.model.domain.rgp.GracePeriodStatus;
@ -84,9 +82,7 @@ public class ResaveEntityActionTest {
} }
private void runAction( private void runAction(
Key<ImmutableObject> resourceKey, String resourceKey, DateTime requestedTime, ImmutableSortedSet<DateTime> resaveTimes) {
DateTime requestedTime,
ImmutableSortedSet<DateTime> resaveTimes) {
ResaveEntityAction action = ResaveEntityAction action =
new ResaveEntityAction( new ResaveEntityAction(
resourceKey, requestedTime, resaveTimes, asyncTaskEnqueuer, response); resourceKey, requestedTime, resaveTimes, asyncTaskEnqueuer, response);
@ -110,7 +106,10 @@ public class ResaveEntityActionTest {
DateTime.parse("2017-01-02T10:11:00Z")); DateTime.parse("2017-01-02T10:11:00Z"));
clock.advanceOneMilli(); clock.advanceOneMilli();
assertThat(domain.getCurrentSponsorRegistrarId()).isEqualTo("TheRegistrar"); 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); DomainBase resavedDomain = loadByEntity(domain);
assertThat(resavedDomain.getCurrentSponsorRegistrarId()).isEqualTo("NewRegistrar"); assertThat(resavedDomain.getCurrentSponsorRegistrarId()).isEqualTo("NewRegistrar");
verify(response).setPayload("Entity re-saved."); verify(response).setPayload("Entity re-saved.");
@ -137,7 +136,10 @@ public class ResaveEntityActionTest {
DateTime requestedTime = clock.nowUtc(); DateTime requestedTime = clock.nowUtc();
assertThat(domain.getGracePeriods()).isNotEmpty(); 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); DomainBase resavedDomain = loadByEntity(domain);
assertThat(resavedDomain.getGracePeriods()).isEmpty(); assertThat(resavedDomain.getGracePeriods()).isEmpty();

View file

@ -159,19 +159,19 @@ class VKeyTest {
/** Test stringify() with vkey created via different ways. */ /** Test stringify() with vkey created via different ways. */
@Test @Test
void testStringify_sqlOnlyVKey() throws Exception { void testStringify_sqlOnlyVKey() {
assertThat(VKey.createSql(TestObject.class, "foo").stringify()) assertThat(VKey.createSql(TestObject.class, "foo").stringify())
.isEqualTo("kind:TestObject@sql:rO0ABXQAA2Zvbw"); .isEqualTo("kind:TestObject@sql:rO0ABXQAA2Zvbw");
} }
@Test @Test
void testStringify_ofyOnlyVKey() throws Exception { void testStringify_ofyOnlyVKey() {
assertThat(VKey.createOfy(TestObject.class, Key.create(TestObject.class, "foo")).stringify()) assertThat(VKey.createOfy(TestObject.class, Key.create(TestObject.class, "foo")).stringify())
.isEqualTo("kind:TestObject@ofy:agR0ZXN0chMLEgpUZXN0T2JqZWN0IgNmb28M"); .isEqualTo("kind:TestObject@ofy:agR0ZXN0chMLEgpUZXN0T2JqZWN0IgNmb28M");
} }
@Test @Test
void testStringify_vkeyFromWebsafeKey() throws Exception { void testStringify_vkeyFromWebsafeKey() {
DomainBase domain = newDomainBase("example.com", "ROID-1", persistActiveContact("contact-1")); DomainBase domain = newDomainBase("example.com", "ROID-1", persistActiveContact("contact-1"));
Key<DomainBase> key = Key.create(domain); Key<DomainBase> key = Key.create(domain);
VKey<DomainBase> vkey = VKey.fromWebsafeKey(key.getString()); VKey<DomainBase> vkey = VKey.fromWebsafeKey(key.getString());
@ -183,7 +183,7 @@ class VKeyTest {
} }
@Test @Test
void testStringify_sqlAndOfyVKey() throws Exception { void testStringify_sqlAndOfyVKey() {
assertThat( assertThat(
VKey.create(TestObject.class, "foo", Key.create(TestObject.create("foo"))).stringify()) VKey.create(TestObject.class, "foo", Key.create(TestObject.create("foo"))).stringify())
.isEqualTo( .isEqualTo(
@ -192,7 +192,7 @@ class VKeyTest {
} }
@Test @Test
void testStringify_asymmetricVKey() throws Exception { void testStringify_asymmetricVKey() {
assertThat( assertThat(
VKey.create(TestObject.class, "test", Key.create(TestObject.create("foo"))).stringify()) VKey.create(TestObject.class, "test", Key.create(TestObject.create("foo"))).stringify())
.isEqualTo( .isEqualTo(
@ -202,19 +202,19 @@ class VKeyTest {
/** Test create() via different vkey string representations. */ /** Test create() via different vkey string representations. */
@Test @Test
void testCreate_stringifedVKey_sqlOnlyVKeyString() throws Exception { void testCreate_stringifedVKey_sqlOnlyVKeyString() {
assertThat(VKey.create("kind:TestObject@sql:rO0ABXQAA2Zvbw")) assertThat(VKey.create("kind:TestObject@sql:rO0ABXQAA2Zvbw"))
.isEqualTo(VKey.createSql(TestObject.class, "foo")); .isEqualTo(VKey.createSql(TestObject.class, "foo"));
} }
@Test @Test
void testCreate_stringifedVKey_ofyOnlyVKeyString() throws Exception { void testCreate_stringifedVKey_ofyOnlyVKeyString() {
assertThat(VKey.create("kind:TestObject@ofy:agR0ZXN0chMLEgpUZXN0T2JqZWN0IgNmb28M")) assertThat(VKey.create("kind:TestObject@ofy:agR0ZXN0chMLEgpUZXN0T2JqZWN0IgNmb28M"))
.isEqualTo(VKey.createOfy(TestObject.class, Key.create(TestObject.class, "foo"))); .isEqualTo(VKey.createOfy(TestObject.class, Key.create(TestObject.class, "foo")));
} }
@Test @Test
void testCreate_stringifedVKey_asymmetricVKeyString() throws Exception { void testCreate_stringifedVKey_asymmetricVKeyString() {
assertThat( assertThat(
VKey.create( VKey.create(
"kind:TestObject@sql:rO0ABXQABHRlc3Q@ofy:agR0ZXN0cjELEg9Fb" "kind:TestObject@sql:rO0ABXQABHRlc3Q@ofy:agR0ZXN0cjELEg9Fb"
@ -223,7 +223,7 @@ class VKeyTest {
} }
@Test @Test
void testCreate_stringifedVKey_sqlAndOfyVKeyString() throws Exception { void testCreate_stringifedVKey_sqlAndOfyVKeyString() {
assertThat( assertThat(
VKey.create( VKey.create(
"kind:TestObject@sql:rO0ABXQAA2Zvbw@ofy:agR0ZXN0cjELEg9Fbn" "kind:TestObject@sql:rO0ABXQAA2Zvbw@ofy:agR0ZXN0cjELEg9Fbn"
@ -232,7 +232,7 @@ class VKeyTest {
} }
@Test @Test
void testCreate_stringifyVkey_fromWebsafeKey() throws Exception { void testCreate_stringifyVkey_fromWebsafeKey() {
assertThat( assertThat(
VKey.create( VKey.create(
"kind:DomainBase@sql:rO0ABXQABlJPSUQtMQ" "kind:DomainBase@sql:rO0ABXQABlJPSUQtMQ"
@ -245,13 +245,13 @@ class VKeyTest {
} }
@Test @Test
void testCreate_stringifedVKey_websafeKey() throws Exception { void testCreate_stringifedVKey_websafeKey() {
assertThat(VKey.create("agR0ZXN0chYLEgpEb21haW5CYXNlIgZST0lELTEM")) assertThat(VKey.create("agR0ZXN0chYLEgpEb21haW5CYXNlIgZST0lELTEM"))
.isEqualTo(VKey.fromWebsafeKey("agR0ZXN0chYLEgpEb21haW5CYXNlIgZST0lELTEM")); .isEqualTo(VKey.fromWebsafeKey("agR0ZXN0chYLEgpEb21haW5CYXNlIgZST0lELTEM"));
} }
@Test @Test
void testCreate_invalidStringifiedVKey_failure() throws Exception { void testCreate_invalidStringifiedVKey_failure() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows( assertThrows(
IllegalArgumentException.class, () -> VKey.create("kind:TestObject@sq:l@ofya:bc")); IllegalArgumentException.class, () -> VKey.create("kind:TestObject@sq:l@ofya:bc"));
@ -261,7 +261,7 @@ class VKeyTest {
} }
@Test @Test
void testCreate_invalidOfyKeyString_failure() throws Exception { void testCreate_invalidOfyKeyString_failure() {
IllegalArgumentException thrown = IllegalArgumentException thrown =
assertThrows(IllegalArgumentException.class, () -> VKey.create("invalid")); assertThrows(IllegalArgumentException.class, () -> VKey.create("invalid"));
assertThat(thrown).hasMessageThat().contains("Could not parse Reference"); assertThat(thrown).hasMessageThat().contains("Could not parse Reference");
@ -269,21 +269,21 @@ class VKeyTest {
/** Test stringify() then create() flow. */ /** Test stringify() then create() flow. */
@Test @Test
void testStringifyThenCreate_sqlOnlyVKey_testObject_stringKey_success() throws Exception { void testStringifyThenCreate_sqlOnlyVKey_testObject_stringKey_success() {
VKey<TestObject> vkey = VKey.createSql(TestObject.class, "foo"); VKey<TestObject> vkey = VKey.createSql(TestObject.class, "foo");
VKey<TestObject> newVkey = VKey.create(vkey.stringify()); VKey<TestObject> newVkey = VKey.create(vkey.stringify());
assertThat(newVkey).isEqualTo(vkey); assertThat(newVkey).isEqualTo(vkey);
} }
@Test @Test
void testStringifyThenCreate_sqlOnlyVKey_testObject_longKey_success() throws Exception { void testStringifyThenCreate_sqlOnlyVKey_testObject_longKey_success() {
VKey<TestObject> vkey = VKey.createSql(TestObject.class, (long) 12345); VKey<TestObject> vkey = VKey.createSql(TestObject.class, (long) 12345);
VKey<TestObject> newVkey = VKey.create(vkey.stringify()); VKey<TestObject> newVkey = VKey.create(vkey.stringify());
assertThat(newVkey).isEqualTo(vkey); assertThat(newVkey).isEqualTo(vkey);
} }
@Test @Test
void testCreate_createFromExistingOfyKey_success() throws Exception { void testCreate_createFromExistingOfyKey_success() {
String keyString = String keyString =
Key.create(newDomainBase("example.com", "ROID-1", persistActiveContact("contact-1"))) Key.create(newDomainBase("example.com", "ROID-1", persistActiveContact("contact-1")))
.getString(); .getString();
@ -291,34 +291,34 @@ class VKeyTest {
} }
@Test @Test
void testStringifyThenCreate_ofyOnlyVKey_testObject_success() throws Exception { void testStringifyThenCreate_ofyOnlyVKey_testObject_success() {
VKey<TestObject> vkey = VKey<TestObject> vkey =
VKey.createOfy(TestObject.class, Key.create(TestObject.class, "tmpKey")); VKey.createOfy(TestObject.class, Key.create(TestObject.class, "tmpKey"));
assertThat(VKey.create(vkey.stringify())).isEqualTo(vkey); assertThat(VKey.create(vkey.stringify())).isEqualTo(vkey);
} }
@Test @Test
void testStringifyThenCreate_ofyOnlyVKey_testObject_websafeString_success() throws Exception { void testStringifyThenCreate_ofyOnlyVKey_testObject_websafeString_success() {
VKey<TestObject> vkey = VKey.fromWebsafeKey(Key.create(TestObject.create("foo")).getString()); VKey<TestObject> vkey = VKey.fromWebsafeKey(Key.create(TestObject.create("foo")).getString());
assertThat(VKey.create(vkey.stringify())).isEqualTo(vkey); assertThat(VKey.create(vkey.stringify())).isEqualTo(vkey);
} }
@Test @Test
void testStringifyThenCreate_sqlAndOfyVKey_success() throws Exception { void testStringifyThenCreate_sqlAndOfyVKey_success() {
VKey<TestObject> vkey = VKey<TestObject> vkey =
VKey.create(TestObject.class, "foo", Key.create(TestObject.create("foo"))); VKey.create(TestObject.class, "foo", Key.create(TestObject.create("foo")));
assertThat(VKey.create(vkey.stringify())).isEqualTo(vkey); assertThat(VKey.create(vkey.stringify())).isEqualTo(vkey);
} }
@Test @Test
void testStringifyThenCreate_asymmetricVKey_success() throws Exception { void testStringifyThenCreate_asymmetricVKey_success() {
VKey<TestObject> vkey = VKey<TestObject> vkey =
VKey.create(TestObject.class, "sqlKey", Key.create(TestObject.create("foo"))); VKey.create(TestObject.class, "sqlKey", Key.create(TestObject.create("foo")));
assertThat(VKey.create(vkey.stringify())).isEqualTo(vkey); assertThat(VKey.create(vkey.stringify())).isEqualTo(vkey);
} }
@Test @Test
void testStringifyThenCreate_symmetricVKey_success() throws Exception { void testStringifyThenCreate_symmetricVKey_success() {
VKey<TestObject> vkey = TestObject.create("foo").key(); VKey<TestObject> vkey = TestObject.create("foo").key();
assertThat(VKey.create(vkey.stringify())).isEqualTo(vkey); assertThat(VKey.create(vkey.stringify())).isEqualTo(vkey);
} }