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

Replace VKey.fromWebsafeKey() with VKey.create(string) (#1414)

Browse source 
* Replace with stringify() and VKey.create(string)

* Convert implicit cases of VKey.fromWebsafeKey(string)

* Convert from Key to VKey to use stringify()

* Modify existing code to show correct string representation of a key

* Use VKey.create(websafeKey) to get ofy key in ResaveEntitiesCommand

* Add TODO note in CommitLogMutation and determine if key string should be modified

* Revert from stringify() to getOfyKey().getString()

* Add bug ids to TODOs
This commit is contained in:
Rachel Guan 2021-11-24 12:14:13 -05:00 • committed by GitHub
parent e4fb083f8a
commit 25d5d3d25e
20 changed files with 54 additions and 55 deletions
Download patch file Download diff file Expand all files Collapse all files

15
core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java
View file

@ -24,10 +24,8 @@ import com.google.appengine.api.taskqueue.TransientFailureException;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.flogger.FluentLogger;
import com.googlecode.objectify.Key;
import google.registry.config.RegistryConfig.Config;
import google.registry.model.EppResource;
import google.registry.model.ImmutableObject;
import google.registry.model.domain.RegistryLock;
import google.registry.model.eppcommon.Trid;
import google.registry.model.host.HostResource;
@ -84,8 +82,7 @@ public final class AsyncTaskEnqueuer {
}
/** Enqueues a task to asynchronously re-save an entity at some point in the future. */
public void enqueueAsyncResave(
ImmutableObject entityToResave, DateTime now, DateTime whenToResave) {
public void enqueueAsyncResave(VKey<?> entityToResave, DateTime now, DateTime whenToResave) {
enqueueAsyncResave(entityToResave, now, ImmutableSortedSet.of(whenToResave));
}
@ -96,10 +93,9 @@ public final class AsyncTaskEnqueuer {
* itself to run at the next time if there are remaining re-saves scheduled.
*/
public void enqueueAsyncResave(
ImmutableObject entityToResave, DateTime now, ImmutableSortedSet<DateTime> whenToResave) {
VKey<?> entityKey, DateTime now, ImmutableSortedSet<DateTime> whenToResave) {
DateTime firstResave = whenToResave.first();
checkArgument(isBeforeOrAt(now, firstResave), "Can't enqueue a resave to run in the past");
Key<ImmutableObject> entityKey = Key.create(entityToResave);
Duration etaDuration = new Duration(now, firstResave);
if (etaDuration.isLongerThan(MAX_ASYNC_ETA)) {
logger.atInfo().log(
@ -114,7 +110,7 @@ public final class AsyncTaskEnqueuer {
.method(Method.POST)
.header("Host", backendHostname)
.countdownMillis(etaDuration.getMillis())
.param(PARAM_RESOURCE_KEY, entityKey.getString())
.param(PARAM_RESOURCE_KEY, entityKey.getOfyKey().getString())
.param(PARAM_REQUESTED_TIME, now.toString());
if (whenToResave.size() > 1) {
task.param(PARAM_RESAVE_TIMES, Joiner.on(',').join(whenToResave.tailSet(firstResave, false)));
@ -129,14 +125,13 @@ public final class AsyncTaskEnqueuer {
String requestingRegistrarId,
Trid trid,
boolean isSuperuser) {
Key<EppResource> resourceKey = Key.create(resourceToDelete);
logger.atInfo().log(
"Enqueuing async deletion of %s on behalf of registrar %s.",
resourceKey, requestingRegistrarId);
resourceToDelete.getRepoId(), requestingRegistrarId);
TaskOptions task =
TaskOptions.Builder.withMethod(Method.PULL)
.countdownMillis(asyncDeleteDelay.getMillis())
.param(PARAM_RESOURCE_KEY, resourceKey.getString())
.param(PARAM_RESOURCE_KEY, resourceToDelete.createVKey().getOfyKey().getString())
.param(PARAM_REQUESTING_CLIENT_ID, requestingRegistrarId)
.param(PARAM_SERVER_TRANSACTION_ID, trid.getServerTransactionId())
.param(PARAM_IS_SUPERUSER, Boolean.toString(isSuperuser))

1
core/src/main/java/google/registry/batch/BatchModule.java
View file

@ -79,6 +79,7 @@ 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<ImmutableObject> provideResourceKey(HttpServletRequest req) {
return Key.create(extractRequiredParameter(req, PARAM_RESOURCE_KEY));
}

9
core/src/main/java/google/registry/batch/DeleteContactsAndHostsAction.java
View file

@ -523,18 +523,19 @@ public class DeleteContactsAndHostsAction implements Runnable {
static DeletionRequest createFromTask(TaskHandle task, DateTime now)
throws Exception {
ImmutableMap<String, String> params = ImmutableMap.copyOf(task.extractParams());
Key<EppResource> resourceKey =
Key.create(
VKey<EppResource> resourceKey =
VKey.create(
checkNotNull(params.get(PARAM_RESOURCE_KEY), "Resource to delete not specified"));
EppResource resource =
checkNotNull(
auditedOfy().load().key(resourceKey).now(), "Resource to delete doesn't exist");
auditedOfy().load().key(resourceKey.getOfyKey()).now(),
"Resource to delete doesn't exist");
checkState(
resource instanceof ContactResource || resource instanceof HostResource,
"Cannot delete a %s via this action",
resource.getClass().getSimpleName());
return new AutoValue_DeleteContactsAndHostsAction_DeletionRequest.Builder()
.setKey(resourceKey)
.setKey(resourceKey.getOfyKey())
.setLastUpdateTime(resource.getUpdateTimestamp().getTimestamp())
.setRequestingClientId(
checkNotNull(

3
core/src/main/java/google/registry/batch/RefreshDnsOnHostRenameAction.java
View file

@ -353,8 +353,7 @@ public class RefreshDnsOnHostRenameAction implements Runnable {
static DnsRefreshRequest createFromTask(TaskHandle task, DateTime now) throws Exception {
ImmutableMap<String, String> params = ImmutableMap.copyOf(task.extractParams());
VKey<HostResource> hostKey =
VKey.fromWebsafeKey(
checkNotNull(params.get(PARAM_HOST_KEY), "Host to refresh not specified"));
VKey.create(checkNotNull(params.get(PARAM_HOST_KEY), "Host to refresh not specified"));
HostResource host =
tm().transact(() -> tm().loadByKeyIfPresent(hostKey))
.orElseThrow(() -> new NoSuchElementException("Host to refresh doesn't exist"));

4
core/src/main/java/google/registry/batch/ResaveEntityAction.java
View file

@ -76,13 +76,15 @@ 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));
tm().put(
(entity instanceof EppResource)
? ((EppResource) entity).cloneProjectedAtTime(tm().getTransactionTime())
: entity);
if (!resaveTimes.isEmpty()) {
asyncTaskEnqueuer.enqueueAsyncResave(entity, requestedTime, resaveTimes);
asyncTaskEnqueuer.enqueueAsyncResave(
VKey.from(resourceKey), requestedTime, resaveTimes);
}
});
response.setPayload("Entity re-saved.");

2
core/src/main/java/google/registry/flows/domain/DomainDeleteFlow.java
View file

@ -187,7 +187,7 @@ public final class DomainDeleteFlow implements TransactionalFlow {
} else {
DateTime redemptionTime = now.plus(redemptionGracePeriodLength);
asyncTaskEnqueuer.enqueueAsyncResave(
existingDomain, now, ImmutableSortedSet.of(redemptionTime, deletionTime));
existingDomain.createVKey(), now, ImmutableSortedSet.of(redemptionTime, deletionTime));
builder
.setDeletionTime(deletionTime)
.setStatusValues(ImmutableSet.of(StatusValue.PENDING_DELETE))

2
core/src/main/java/google/registry/flows/domain/DomainTransferRequestFlow.java
View file

@ -235,7 +235,7 @@ public final class DomainTransferRequestFlow implements TransactionalFlow {
.build();
DomainHistory domainHistory = buildDomainHistory(newDomain, registry, now, period);
asyncTaskEnqueuer.enqueueAsyncResave(newDomain, now, automaticTransferTime);
asyncTaskEnqueuer.enqueueAsyncResave(newDomain.createVKey(), now, automaticTransferTime);
tm().putAll(
new ImmutableSet.Builder<>()
.add(newDomain, domainHistory, requestPollMessage)

1
core/src/main/java/google/registry/model/index/EppResourceIndex.java
View file

@ -64,6 +64,7 @@ public class EppResourceIndex extends BackupGroupRoot implements DatastoreOnlyEn
EppResourceIndex instance = instantiate(EppResourceIndex.class);
instance.reference = resourceKey;
instance.kind = resourceKey.getKind();
// TODO(b/207368050): figure out if this value has ever been used other than test cases
instance.id = resourceKey.getString(); // creates a web-safe key string
instance.bucket = bucket;
return instance;

3
core/src/main/java/google/registry/model/ofy/CommitLogMutation.java
View file

@ -82,6 +82,7 @@ public class CommitLogMutation extends ImmutableObject implements DatastoreOnlyE
com.google.appengine.api.datastore.Entity rawEntity) {
CommitLogMutation instance = new CommitLogMutation();
instance.parent = checkNotNull(parent);
// TODO(b/207516684): figure out if this should be converted to a vkey string via stringify()
// Creates a web-safe key string.
instance.entityKey = KeyFactory.keyToString(rawEntity.getKey());
instance.entityProtoBytes = convertToPb(rawEntity).toByteArray();
@ -91,6 +92,8 @@ public class CommitLogMutation extends ImmutableObject implements DatastoreOnlyE
/** Returns the key of a mutation based on the {@code entityKey} of the entity it stores. */
public static
Key<CommitLogMutation> createKey(Key<CommitLogManifest> parent, Key<?> entityKey) {
// TODO(b/207516684): figure out if the return type needs to be VKey and
// if the string used to create a key should remain the same
return Key.create(parent, CommitLogMutation.class, entityKey.getString());
}
}

3
core/src/main/java/google/registry/tools/GetEppResourceCommand.java
View file

@ -19,7 +19,6 @@ import static org.joda.time.DateTimeZone.UTC;
import com.beust.jcommander.Parameter;
import com.beust.jcommander.Parameters;
import com.googlecode.objectify.Key;
import google.registry.model.EppResource;
import java.util.Optional;
import org.joda.time.DateTime;
@ -56,7 +55,7 @@ abstract class GetEppResourceCommand implements CommandWithRemoteApi {
? String.format(
"%s\n\nWebsafe key: %s",
expand ? resource.get().toHydratedString() : resource.get(),
Key.create(resource.get()).getString())
resource.get().createVKey().getOfyKey().getString())
: String.format("%s '%s' does not exist or is deleted\n", resourceType, uniqueId));
}

12
core/src/main/java/google/registry/tools/GetResourceByKeyCommand.java
View file

@ -19,8 +19,8 @@ import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import com.beust.jcommander.Parameter;
import com.beust.jcommander.Parameters;
import com.googlecode.objectify.Key;
import google.registry.model.EppResource;
import google.registry.persistence.VKey;
import java.util.List;
/**
@ -40,15 +40,15 @@ final class GetResourceByKeyCommand implements CommandWithRemoteApi {
boolean expand;
@Override
public void run() {
public void run() throws Exception {
for (String keyString : mainParameters) {
System.out.println("\n");
Key<EppResource> resourceKey =
checkNotNull(Key.create(keyString), "Could not parse key string: " + keyString);
VKey<EppResource> resourceKey =
checkNotNull(VKey.create(keyString), "Could not parse key string: " + keyString);
EppResource resource =
checkNotNull(
auditedOfy().load().key(resourceKey).now(),
"Could not load resource for key: " + resourceKey);
auditedOfy().load().key(resourceKey.getOfyKey()).now(),
"Could not load resource for key: " + resourceKey.getOfyKey());
System.out.println(expand ? resource.toHydratedString() : resource.toString());
}
}

7
core/src/main/java/google/registry/tools/ResaveEntitiesCommand.java
View file

@ -19,8 +19,8 @@ import static google.registry.model.ofy.ObjectifyService.auditedOfy;
import com.beust.jcommander.Parameter;
import com.beust.jcommander.Parameters;
import com.googlecode.objectify.Key;
import google.registry.model.ImmutableObject;
import google.registry.persistence.VKey;
import java.util.List;
/**
@ -37,15 +37,16 @@ public final class ResaveEntitiesCommand extends MutatingCommand {
/** The number of resaves to do in a single transaction. */
private static final int BATCH_SIZE = 10;
// TODO(b/207376744): figure out if there's a guide that shows how a websafe key should look like
@Parameter(description = "Websafe keys", required = true)
List<String> mainParameters;
@Override
protected void init() {
protected void init() throws Exception {
for (List<String> batch : partition(mainParameters, BATCH_SIZE)) {
for (String websafeKey : batch) {
ImmutableObject entity =
auditedOfy().load().key(Key.<ImmutableObject>create(websafeKey)).now();
(ImmutableObject) auditedOfy().load().key(VKey.create(websafeKey).getOfyKey()).now();
stageEntityChange(entity, entity);
}
flushTransaction();

13
core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java
View file

@ -35,7 +35,6 @@ import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.flogger.LoggerConfig;
import com.googlecode.objectify.Key;
import google.registry.model.contact.ContactResource;
import google.registry.model.domain.RegistryLock;
import google.registry.testing.AppEngineExtension;
@ -95,7 +94,8 @@ public class AsyncTaskEnqueuerTest {
@Test
void test_enqueueAsyncResave_success() {
ContactResource contact = persistActiveContact("jd23456");
asyncTaskEnqueuer.enqueueAsyncResave(contact, clock.nowUtc(), clock.nowUtc().plusDays(5));
asyncTaskEnqueuer.enqueueAsyncResave(
contact.createVKey(), clock.nowUtc(), clock.nowUtc().plusDays(5));
assertTasksEnqueued(
QUEUE_ASYNC_ACTIONS,
new TaskMatcher()
@ -103,7 +103,7 @@ public class AsyncTaskEnqueuerTest {
.method("POST")
.header("Host", "backend.hostname.fake")
.header("content-type", "application/x-www-form-urlencoded")
.param(PARAM_RESOURCE_KEY, Key.create(contact).getString())
.param(PARAM_RESOURCE_KEY, contact.createVKey().getOfyKey().getString())
.param(PARAM_REQUESTED_TIME, clock.nowUtc().toString())
.etaDelta(
standardDays(5).minus(standardSeconds(30)),
@ -115,7 +115,7 @@ public class AsyncTaskEnqueuerTest {
ContactResource contact = persistActiveContact("jd23456");
DateTime now = clock.nowUtc();
asyncTaskEnqueuer.enqueueAsyncResave(
contact,
contact.createVKey(),
now,
ImmutableSortedSet.of(now.plusHours(24), now.plusHours(50), now.plusHours(75)));
assertTasksEnqueued(
@ -125,7 +125,7 @@ public class AsyncTaskEnqueuerTest {
.method("POST")
.header("Host", "backend.hostname.fake")
.header("content-type", "application/x-www-form-urlencoded")
.param(PARAM_RESOURCE_KEY, Key.create(contact).getString())
.param(PARAM_RESOURCE_KEY, contact.createVKey().getOfyKey().getString())
.param(PARAM_REQUESTED_TIME, now.toString())
.param(PARAM_RESAVE_TIMES, "2015-05-20T14:34:56.000Z,2015-05-21T15:34:56.000Z")
.etaDelta(
@ -137,7 +137,8 @@ public class AsyncTaskEnqueuerTest {
@Test
void test_enqueueAsyncResave_ignoresTasksTooFarIntoFuture() {
ContactResource contact = persistActiveContact("jd23456");
asyncTaskEnqueuer.enqueueAsyncResave(contact, clock.nowUtc(), clock.nowUtc().plusDays(31));
asyncTaskEnqueuer.enqueueAsyncResave(
contact.createVKey(), clock.nowUtc(), clock.nowUtc().plusDays(31));
assertNoTasksEnqueued(QUEUE_ASYNC_ACTIONS);
assertLogMessage(logHandler, Level.INFO, "Ignoring async re-save");
}

5
core/src/test/java/google/registry/batch/DeleteContactsAndHostsActionTest.java
View file

@ -64,7 +64,6 @@ import com.google.appengine.api.taskqueue.TaskOptions.Method;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.googlecode.objectify.Key;
import google.registry.batch.AsyncTaskMetrics.OperationResult;
import google.registry.batch.AsyncTaskMetrics.OperationType;
import google.registry.batch.DeleteContactsAndHostsAction.DeleteEppResourceReducer;
@ -497,7 +496,7 @@ public class DeleteContactsAndHostsActionTest
QUEUE_ASYNC_DELETE,
new TaskMatcher()
.etaDelta(standardHours(23), standardHours(25))
.param("resourceKey", Key.create(contactNotSaved).getString())
.param("resourceKey", contactNotSaved.createVKey().getOfyKey().getString())
.param("requestingClientId", "TheRegistrar")
.param("clientTransactionId", "fakeClientTrid")
.param("serverTransactionId", "fakeServerTrid")
@ -505,7 +504,7 @@ public class DeleteContactsAndHostsActionTest
.param("requestedTime", timeBeforeRun.toString()),
new TaskMatcher()
.etaDelta(standardHours(23), standardHours(25))
.param("resourceKey", Key.create(hostNotSaved).getString())
.param("resourceKey", hostNotSaved.createVKey().getOfyKey().getString())
.param("requestingClientId", "TheRegistrar")
.param("clientTransactionId", "fakeClientTrid")
.param("serverTransactionId", "fakeServerTrid")

3
core/src/test/java/google/registry/batch/RefreshDnsOnHostRenameActionTest.java
View file

@ -44,7 +44,6 @@ import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
import com.googlecode.objectify.Key;
import google.registry.batch.AsyncTaskMetrics.OperationResult;
import google.registry.batch.RefreshDnsOnHostRenameAction.RefreshDnsOnHostRenameReducer;
import google.registry.dns.DnsQueue;
@ -238,7 +237,7 @@ public class RefreshDnsOnHostRenameActionTest
QUEUE_ASYNC_HOST_RENAME,
new TaskMatcher()
.etaDelta(standardHours(23), standardHours(25))
.param("hostKey", Key.create(host).getString()));
.param("hostKey", host.createVKey().getOfyKey().getString()));
assertThat(acquireLock()).isPresent();
}

2
core/src/test/java/google/registry/batch/ResaveEntityActionTest.java
View file

@ -148,7 +148,7 @@ public class ResaveEntityActionTest {
.method("POST")
.header("Host", "backend.hostname.fake")
.header("content-type", "application/x-www-form-urlencoded")
.param(PARAM_RESOURCE_KEY, Key.create(resavedDomain).getString())
.param(PARAM_RESOURCE_KEY, resavedDomain.createVKey().getOfyKey().getString())
.param(PARAM_REQUESTED_TIME, requestedTime.toString())
.etaDelta(
standardDays(5).minus(standardSeconds(30)),

15
core/src/test/java/google/registry/flows/ResourceFlowTestCase.java
View file

@ -154,13 +154,14 @@ public abstract class ResourceFlowTestCase<F extends Flow, R extends EppResource
/** Asserts the presence of a single enqueued async contact or host deletion */
protected <T extends EppResource> void assertAsyncDeletionTaskEnqueued(
T resource, String requestingClientId, Trid trid, boolean isSuperuser) {
TaskMatcher expected = new TaskMatcher()
.etaDelta(Duration.standardSeconds(75), Duration.standardSeconds(105)) // expected: 90
.param("resourceKey", Key.create(resource).getString())
.param("requestingClientId", requestingClientId)
.param("serverTransactionId", trid.getServerTransactionId())
.param("isSuperuser", Boolean.toString(isSuperuser))
.param("requestedTime", clock.nowUtc().toString());
TaskMatcher expected =
new TaskMatcher()
.etaDelta(Duration.standardSeconds(75), Duration.standardSeconds(105)) // expected: 90
.param("resourceKey", resource.createVKey().getOfyKey().getString())
.param("requestingClientId", requestingClientId)
.param("serverTransactionId", trid.getServerTransactionId())
.param("isSuperuser", Boolean.toString(isSuperuser))
.param("requestedTime", clock.nowUtc().toString());
trid.getClientTransactionId()
.ifPresent(clTrid -> expected.param("clientTransactionId", clTrid));
assertTasksEnqueued("async-delete-pull", expected);

3
core/src/test/java/google/registry/flows/domain/DomainDeleteFlowTest.java
View file

@ -66,7 +66,6 @@ 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.googlecode.objectify.Key;
import google.registry.batch.ResaveEntityAction;
import google.registry.flows.EppException;
import google.registry.flows.EppException.UnimplementedExtensionException;
@ -311,7 +310,7 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
.method("POST")
.header("Host", "backend.hostname.fake")
.header("content-type", "application/x-www-form-urlencoded")
.param(PARAM_RESOURCE_KEY, Key.create(domain).getString())
.param(PARAM_RESOURCE_KEY, domain.createVKey().getOfyKey().getString())
.param(PARAM_REQUESTED_TIME, clock.nowUtc().toString())
.param(PARAM_RESAVE_TIMES, clock.nowUtc().plusDays(5).toString())
.etaDelta(when.minus(standardSeconds(30)), when.plus(standardSeconds(30))));

3
core/src/test/java/google/registry/flows/domain/DomainTransferRequestFlowTest.java
View file

@ -58,7 +58,6 @@ import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.common.collect.Streams;
import com.googlecode.objectify.Key;
import google.registry.batch.ResaveEntityAction;
import google.registry.flows.EppException;
import google.registry.flows.EppRequestSource;
@ -518,7 +517,7 @@ class DomainTransferRequestFlowTest
.method("POST")
.header("Host", "backend.hostname.fake")
.header("content-type", "application/x-www-form-urlencoded")
.param(PARAM_RESOURCE_KEY, Key.create(domain).getString())
.param(PARAM_RESOURCE_KEY, domain.createVKey().getOfyKey().getString())
.param(PARAM_REQUESTED_TIME, clock.nowUtc().toString())
.etaDelta(
registry.getAutomaticTransferLength().minus(standardSeconds(30)),

3
core/src/test/java/google/registry/flows/host/HostUpdateFlowTest.java
View file

@ -46,7 +46,6 @@ import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.net.InetAddresses;
import com.googlecode.objectify.Key;
import google.registry.flows.EppException;
import google.registry.flows.EppRequestSource;
import google.registry.flows.ResourceFlowTestCase;
@ -220,7 +219,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, HostResour
assertTasksEnqueued(
QUEUE_ASYNC_HOST_RENAME,
new TaskMatcher()
.param("hostKey", Key.create(renamedHost).getString())
.param("hostKey", renamedHost.createVKey().getOfyKey().getString())
.param("requestedTime", clock.nowUtc().toString()));
}