Replace all existing vkey string to vkey.stringify() (#1430)

* Resolve ResaveEntityAction related conflicts

* Replace string with existing constants

* Remove solved TODOs related to ofy string to new vkey string

* Add a TODO for clean up

* Fix missing annotation
This commit is contained in:
Rachel Guan 2022-01-07 12:11:15 -05:00 committed by GitHub
parent ceade7f954
commit d04b3299aa
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 128 additions and 30 deletions

View file

@ -110,7 +110,7 @@ public final class AsyncTaskEnqueuer {
.method(Method.POST)
.header("Host", backendHostname)
.countdownMillis(etaDuration.getMillis())
.param(PARAM_RESOURCE_KEY, entityKey.getOfyKey().getString())
.param(PARAM_RESOURCE_KEY, entityKey.stringify())
.param(PARAM_REQUESTED_TIME, now.toString());
if (whenToResave.size() > 1) {
task.param(PARAM_RESAVE_TIMES, Joiner.on(',').join(whenToResave.tailSet(firstResave, false)));
@ -131,7 +131,7 @@ public final class AsyncTaskEnqueuer {
TaskOptions task =
TaskOptions.Builder.withMethod(Method.PULL)
.countdownMillis(asyncDeleteDelay.getMillis())
.param(PARAM_RESOURCE_KEY, resourceToDelete.createVKey().getOfyKey().getString())
.param(PARAM_RESOURCE_KEY, resourceToDelete.createVKey().stringify())
.param(PARAM_REQUESTING_CLIENT_ID, requestingRegistrarId)
.param(PARAM_SERVER_TRANSACTION_ID, trid.getServerTransactionId())
.param(PARAM_IS_SUPERUSER, Boolean.toString(isSuperuser))
@ -148,7 +148,7 @@ public final class AsyncTaskEnqueuer {
addTaskToQueueWithRetry(
asyncDnsRefreshPullQueue,
TaskOptions.Builder.withMethod(Method.PULL)
.param(PARAM_HOST_KEY, hostKey.getOfyKey().getString())
.param(PARAM_HOST_KEY, hostKey.stringify())
.param(PARAM_REQUESTED_TIME, now.toString()));
}

View file

@ -27,6 +27,7 @@ import google.registry.model.EppResource;
import google.registry.model.annotations.DeleteAfterMigration;
import google.registry.model.annotations.ReportedOn;
import google.registry.model.replay.DatastoreOnlyEntity;
import google.registry.persistence.VKey;
/** An index that allows for quick enumeration of all EppResource entities (e.g. via map reduce). */
@ReportedOn
@ -66,8 +67,9 @@ 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
// creates a web-safe key string, this value is never used
// TODO(b/211785379): remove unused id
instance.id = VKey.from(resourceKey).stringify();
instance.bucket = bucket;
return instance;
}

View file

@ -144,7 +144,7 @@ public class VKey<T> extends ImmutableObject implements Serializable {
*/
public static <T> VKey<T> create(String keyString) {
if (!keyString.startsWith(CLASS_TYPE + KV_SEPARATOR)) {
// to handle the existing ofy key string
// handle the existing ofy key string
return fromWebsafeKey(keyString);
} else {
ImmutableMap<String, String> kvs =
@ -307,6 +307,7 @@ public class VKey<T> extends ImmutableObject implements Serializable {
if (maybeGetSqlKey().isPresent()) {
key += DELIMITER + SQL_LOOKUP_KEY + KV_SEPARATOR + SerializeUtils.stringify(getSqlKey());
}
// getString() method returns a Base64 encoded web safe of ofy key
if (maybeGetOfyKey().isPresent()) {
key += DELIMITER + OFY_LOOKUP_KEY + KV_SEPARATOR + getOfyKey().getString();
}

View file

@ -55,7 +55,7 @@ abstract class GetEppResourceCommand implements CommandWithRemoteApi {
? String.format(
"%s\n\nWebsafe key: %s",
expand ? resource.get().toHydratedString() : resource.get(),
resource.get().createVKey().getOfyKey().getString())
resource.get().createVKey().stringify())
: String.format("%s '%s' does not exist or is deleted\n", resourceType, uniqueId));
}

View file

@ -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, contact.createVKey().getOfyKey().getString())
.param(PARAM_RESOURCE_KEY, contact.createVKey().stringify())
.param(PARAM_REQUESTED_TIME, clock.nowUtc().toString())
.etaDelta(
standardDays(5).minus(standardSeconds(30)),
@ -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, contact.createVKey().getOfyKey().getString())
.param(PARAM_RESOURCE_KEY, contact.createVKey().stringify())
.param(PARAM_REQUESTED_TIME, now.toString())
.param(PARAM_RESAVE_TIMES, "2015-05-20T14:34:56.000Z,2015-05-21T15:34:56.000Z")
.etaDelta(

View file

@ -18,6 +18,7 @@ import static com.google.appengine.api.taskqueue.QueueFactory.getQueue;
import static com.google.common.collect.MoreCollectors.onlyElement;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESOURCE_KEY;
import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_DELETE;
import static google.registry.batch.AsyncTaskMetrics.OperationResult.STALE;
import static google.registry.model.EppResourceUtils.loadByForeignKey;
@ -496,7 +497,7 @@ public class DeleteContactsAndHostsActionTest
QUEUE_ASYNC_DELETE,
new TaskMatcher()
.etaDelta(standardHours(23), standardHours(25))
.param("resourceKey", contactNotSaved.createVKey().getOfyKey().getString())
.param(PARAM_RESOURCE_KEY, contactNotSaved.createVKey().stringify())
.param("requestingClientId", "TheRegistrar")
.param("clientTransactionId", "fakeClientTrid")
.param("serverTransactionId", "fakeServerTrid")
@ -504,7 +505,7 @@ public class DeleteContactsAndHostsActionTest
.param("requestedTime", timeBeforeRun.toString()),
new TaskMatcher()
.etaDelta(standardHours(23), standardHours(25))
.param("resourceKey", hostNotSaved.createVKey().getOfyKey().getString())
.param(PARAM_RESOURCE_KEY, hostNotSaved.createVKey().stringify())
.param("requestingClientId", "TheRegistrar")
.param("clientTransactionId", "fakeClientTrid")
.param("serverTransactionId", "fakeServerTrid")

View file

@ -145,7 +145,7 @@ public class RefreshDnsOnHostRenameActionTest
assertTasksEnqueued(
QUEUE_ASYNC_HOST_RENAME,
new TaskMatcher()
.param(PARAM_HOST_KEY, host.createVKey().getOfyKey().getString())
.param(PARAM_HOST_KEY, host.createVKey().stringify())
.param(PARAM_REQUESTED_TIME, timeEnqueued.toString()));
verify(action.asyncTaskMetrics).recordDnsRefreshBatchSize(1L);
verifyNoMoreInteractions(action.asyncTaskMetrics);
@ -237,7 +237,7 @@ public class RefreshDnsOnHostRenameActionTest
QUEUE_ASYNC_HOST_RENAME,
new TaskMatcher()
.etaDelta(standardHours(23), standardHours(25))
.param("hostKey", host.createVKey().getOfyKey().getString()));
.param(PARAM_HOST_KEY, host.createVKey().stringify()));
assertThat(acquireLock()).isPresent();
}

View file

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

View file

@ -16,6 +16,7 @@ package google.registry.flows;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.batch.AsyncTaskEnqueuer.PARAM_RESOURCE_KEY;
import static google.registry.model.EppResourceUtils.loadByForeignKey;
import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects;
import static google.registry.model.ofy.ObjectifyService.auditedOfy;
@ -146,7 +147,7 @@ public abstract class ResourceFlowTestCase<F extends Flow, R extends EppResource
TaskMatcher expected =
new TaskMatcher()
.etaDelta(Duration.standardSeconds(75), Duration.standardSeconds(105)) // expected: 90
.param("resourceKey", resource.createVKey().getOfyKey().getString())
.param(PARAM_RESOURCE_KEY, resource.createVKey().stringify())
.param("requestingClientId", requestingClientId)
.param("serverTransactionId", trid.getServerTransactionId())
.param("isSuperuser", Boolean.toString(isSuperuser))

View file

@ -321,7 +321,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, domain.createVKey().getOfyKey().getString())
.param(PARAM_RESOURCE_KEY, domain.createVKey().stringify())
.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))));

View file

@ -521,7 +521,7 @@ class DomainTransferRequestFlowTest
.method("POST")
.header("Host", "backend.hostname.fake")
.header("content-type", "application/x-www-form-urlencoded")
.param(PARAM_RESOURCE_KEY, domain.createVKey().getOfyKey().getString())
.param(PARAM_RESOURCE_KEY, domain.createVKey().stringify())
.param(PARAM_REQUESTED_TIME, clock.nowUtc().toString())
.etaDelta(
registry.getAutomaticTransferLength().minus(standardSeconds(30)),

View file

@ -16,6 +16,7 @@ package google.registry.flows.host;
import static com.google.common.base.Strings.nullToEmpty;
import static com.google.common.truth.Truth.assertThat;
import static google.registry.batch.AsyncTaskEnqueuer.PARAM_HOST_KEY;
import static google.registry.batch.AsyncTaskEnqueuer.QUEUE_ASYNC_HOST_RENAME;
import static google.registry.model.EppResourceUtils.loadByForeignKey;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
@ -230,7 +231,7 @@ class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, HostResour
assertTasksEnqueued(
QUEUE_ASYNC_HOST_RENAME,
new TaskMatcher()
.param("hostKey", renamedHost.createVKey().getOfyKey().getString())
.param(PARAM_HOST_KEY, renamedHost.createVKey().stringify())
.param("requestedTime", clock.nowUtc().toString()));
}

View file

@ -29,6 +29,7 @@ import com.googlecode.objectify.annotation.Entity;
import google.registry.model.billing.BillingEvent.OneTime;
import google.registry.model.common.ClassPathManager;
import google.registry.model.domain.DomainBase;
import google.registry.model.host.HostResource;
import google.registry.model.registrar.RegistrarContact;
import google.registry.testing.AppEngineExtension;
import google.registry.testing.TaskQueueHelper.TaskMatcher;
@ -207,6 +208,29 @@ class VKeyTest {
.isEqualTo(VKey.createSql(TestObject.class, "foo"));
}
@Test
void testCreate_stringifiedVKey_resourceKeyFromTaskQueue() throws Exception {
VKey<HostResource> vkeyFromNewWebsafeKey =
VKey.create(
"kind:HostResource@sql:rO0ABXQADzZCQjJGNDc2LUdPT0dMRQ@ofy:ahdzfm"
+ "RvbWFpbi1yZWdpc3RyeS1hbHBoYXIhCxIMSG9zdFJlc291cmNlIg82QkIyRjQ3Ni1HT09HTEUM");
assertThat(vkeyFromNewWebsafeKey.getSqlKey()).isEqualTo("6BB2F476-GOOGLE");
assertThat(vkeyFromNewWebsafeKey.getOfyKey().getString())
.isEqualTo(
"ahdzfmRvbWFpb"
+ "i1yZWdpc3RyeS1hbHBoYXIhCxIMSG9zdFJlc291cmNlIg82QkIyRjQ3Ni1HT09HTEUM");
// the ofy portion of the new vkey string representation was the old vkey string representation
VKey<HostResource> vkeyFromOldWebsafeString =
VKey.fromWebsafeKey(
"ahdzfmRvbW"
+ "Fpbi1yZWdpc3RyeS1hbHBoYXIhCxIMSG9zdFJlc291cmNlIg82QkIyRjQ3Ni1HT09HTEUM");
// the following is assertion is ensure backwork compatibility
assertThat(vkeyFromNewWebsafeKey).isEqualTo(vkeyFromOldWebsafeString);
}
@Test
void testCreate_stringifedVKey_ofyOnlyVKeyString() {
assertThat(VKey.create("kind:TestObject@ofy:agR0ZXN0chMLEgpUZXN0T2JqZWN0IgNmb28M"))

View file

@ -42,7 +42,11 @@ class GetContactCommandTest extends CommandTestCase<GetContactCommand> {
persistActiveContact("sh8013");
runCommand("sh8013");
assertInStdout("contactId=sh8013");
assertInStdout("Websafe key: agR0ZXN0chsLEg9Db250YWN0UmVzb3VyY2UiBjItUk9JRAw");
assertInStdout(
"Websafe key: "
+ "kind:ContactResource"
+ "@sql:rO0ABXQABjItUk9JRA"
+ "@ofy:agR0ZXN0chsLEg9Db250YWN0UmVzb3VyY2UiBjItUk9JRAw");
}
@Test
@ -50,7 +54,11 @@ class GetContactCommandTest extends CommandTestCase<GetContactCommand> {
persistActiveContact("sh8013");
runCommand("sh8013", "--expand");
assertInStdout("contactId=sh8013");
assertInStdout("Websafe key: agR0ZXN0chsLEg9Db250YWN0UmVzb3VyY2UiBjItUk9JRAw");
assertInStdout(
"Websafe key: "
+ "kind:ContactResource"
+ "@sql:rO0ABXQABjItUk9JRA"
+ "@ofy:agR0ZXN0chsLEg9Db250YWN0UmVzb3VyY2UiBjItUk9JRAw");
assertNotInStdout("LiveRef");
}
@ -61,8 +69,16 @@ class GetContactCommandTest extends CommandTestCase<GetContactCommand> {
runCommand("sh8013", "jd1234");
assertInStdout("contactId=sh8013");
assertInStdout("contactId=jd1234");
assertInStdout("Websafe key: agR0ZXN0chsLEg9Db250YWN0UmVzb3VyY2UiBjItUk9JRAw");
assertInStdout("Websafe key: agR0ZXN0chsLEg9Db250YWN0UmVzb3VyY2UiBjMtUk9JRAw");
assertInStdout(
"Websafe key: "
+ "kind:ContactResource"
+ "@sql:rO0ABXQABjItUk9JRA"
+ "@ofy:agR0ZXN0chsLEg9Db250YWN0UmVzb3VyY2UiBjItUk9JRAw");
assertInStdout(
"Websafe key: "
+ "kind:ContactResource"
+ "@sql:rO0ABXQABjItUk9JRA"
+ "@ofy:agR0ZXN0chsLEg9Db250YWN0UmVzb3VyY2UiBjItUk9JRAw");
}
@Test

View file

@ -43,7 +43,11 @@ class GetDomainCommandTest extends CommandTestCase<GetDomainCommand> {
runCommand("example.tld");
assertInStdout("fullyQualifiedDomainName=example.tld");
assertInStdout("contact=Key<?>(ContactResource(\"3-ROID\"))");
assertInStdout("Websafe key: agR0ZXN0chULEgpEb21haW5CYXNlIgUyLVRMRAw");
assertInStdout(
"Websafe key: "
+ "kind:DomainBase"
+ "@sql:rO0ABXQABTItVExE"
+ "@ofy:agR0ZXN0chULEgpEb21haW5CYXNlIgUyLVRMRAw");
}
@Test
@ -52,7 +56,11 @@ class GetDomainCommandTest extends CommandTestCase<GetDomainCommand> {
runCommand("example.tld", "--expand");
assertInStdout("fullyQualifiedDomainName=example.tld");
assertInStdout("contactId=contact1234");
assertInStdout("Websafe key: agR0ZXN0chULEgpEb21haW5CYXNlIgUyLVRMRAw");
assertInStdout(
"Websafe key: "
+ "kind:DomainBase"
+ "@sql:rO0ABXQABTItVExE"
+ "@ofy:agR0ZXN0chULEgpEb21haW5CYXNlIgUyLVRMRAw");
assertNotInStdout("LiveRef");
}
@ -63,8 +71,16 @@ class GetDomainCommandTest extends CommandTestCase<GetDomainCommand> {
runCommand("example.tld", "example2.tld");
assertInStdout("fullyQualifiedDomainName=example.tld");
assertInStdout("fullyQualifiedDomainName=example2.tld");
assertInStdout("Websafe key: agR0ZXN0chULEgpEb21haW5CYXNlIgUyLVRMRAw");
assertInStdout("Websafe key: agR0ZXN0chULEgpEb21haW5CYXNlIgU0LVRMRAw");
assertInStdout(
"Websafe key: "
+ "kind:DomainBase"
+ "@sql:rO0ABXQABTQtVExE"
+ "@ofy:agR0ZXN0chULEgpEb21haW5CYXNlIgU0LVRMRAw");
assertInStdout(
"Websafe key: "
+ "kind:DomainBase"
+ "@sql:rO0ABXQABTQtVExE"
+ "@ofy:agR0ZXN0chULEgpEb21haW5CYXNlIgU0LVRMRAw");
}
@Test

View file

@ -42,7 +42,11 @@ class GetHostCommandTest extends CommandTestCase<GetHostCommand> {
persistActiveHost("ns1.example.tld");
runCommand("ns1.example.tld");
assertInStdout("fullyQualifiedHostName=ns1.example.tld");
assertInStdout("Websafe key: agR0ZXN0chgLEgxIb3N0UmVzb3VyY2UiBjItUk9JRAw");
assertInStdout(
"Websafe key: "
+ "kind:HostResource"
+ "@sql:rO0ABXQABjItUk9JRA"
+ "@ofy:agR0ZXN0chgLEgxIb3N0UmVzb3VyY2UiBjItUk9JRAw");
}
@Test
@ -50,7 +54,11 @@ class GetHostCommandTest extends CommandTestCase<GetHostCommand> {
persistActiveHost("ns1.example.tld");
runCommand("ns1.example.tld", "--expand");
assertInStdout("fullyQualifiedHostName=ns1.example.tld");
assertInStdout("Websafe key: agR0ZXN0chgLEgxIb3N0UmVzb3VyY2UiBjItUk9JRAw");
assertInStdout(
"Websafe key: "
+ "kind:HostResource"
+ "@sql:rO0ABXQABjItUk9JRA"
+ "@ofy:agR0ZXN0chgLEgxIb3N0UmVzb3VyY2UiBjItUk9JRAw");
assertNotInStdout("LiveRef");
}
@ -61,8 +69,16 @@ class GetHostCommandTest extends CommandTestCase<GetHostCommand> {
runCommand("ns1.example.tld", "ns2.example.tld");
assertInStdout("fullyQualifiedHostName=ns1.example.tld");
assertInStdout("fullyQualifiedHostName=ns2.example.tld");
assertInStdout("Websafe key: agR0ZXN0chgLEgxIb3N0UmVzb3VyY2UiBjItUk9JRAw");
assertInStdout("Websafe key: agR0ZXN0chgLEgxIb3N0UmVzb3VyY2UiBjMtUk9JRAw");
assertInStdout(
"Websafe key: "
+ "kind:HostResource"
+ "@sql:rO0ABXQABjItUk9JRA"
+ "@ofy:agR0ZXN0chgLEgxIb3N0UmVzb3VyY2UiBjItUk9JRAw");
assertInStdout(
"Websafe key: "
+ "kind:HostResource"
+ "@sql:rO0ABXQABjItUk9JRA"
+ "@ofy:agR0ZXN0chgLEgxIb3N0UmVzb3VyY2UiBjItUk9JRAw");
}
@Test

View file

@ -52,6 +52,26 @@ class ResaveEntitiesCommandTest extends CommandTestCase<ResaveEntitiesCommand> {
.containsExactlyElementsIn(auditedOfy().load().entities(contact1, contact2).values());
}
@Test
void testSuccess_createsCommitLogs_withNewWebsafeKey() throws Exception {
ContactResource contact1 = persistActiveContact("contact1");
ContactResource contact2 = persistActiveContact("contact2");
deleteEntitiesOfTypes(CommitLogManifest.class, CommitLogMutation.class);
assertThat(auditedOfy().load().type(CommitLogManifest.class).keys()).isEmpty();
assertThat(auditedOfy().load().type(CommitLogMutation.class).keys()).isEmpty();
runCommandForced(contact1.createVKey().stringify(), contact2.createVKey().stringify());
assertThat(auditedOfy().load().type(CommitLogManifest.class).keys()).hasSize(1);
Iterable<ImmutableObject> savedEntities =
transform(
auditedOfy().load().type(CommitLogMutation.class).list(),
mutation -> auditedOfy().load().fromEntity(mutation.getEntity()));
// Reload the contacts before asserting, since their update times will have changed.
auditedOfy().clearSessionCache();
assertThat(savedEntities)
.containsExactlyElementsIn(auditedOfy().load().entities(contact1, contact2).values());
}
@SafeVarargs
private static void deleteEntitiesOfTypes(Class<? extends ImmutableObject>... types) {
for (Class<? extends ImmutableObject> type : types) {