diff --git a/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java b/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java index e044ea0ec..9932a9410 100644 --- a/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java +++ b/core/src/main/java/google/registry/batch/AsyncTaskEnqueuer.java @@ -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())); } diff --git a/core/src/main/java/google/registry/model/index/EppResourceIndex.java b/core/src/main/java/google/registry/model/index/EppResourceIndex.java index 2920af6b8..5a577d9ba 100644 --- a/core/src/main/java/google/registry/model/index/EppResourceIndex.java +++ b/core/src/main/java/google/registry/model/index/EppResourceIndex.java @@ -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; } diff --git a/core/src/main/java/google/registry/persistence/VKey.java b/core/src/main/java/google/registry/persistence/VKey.java index f992a6223..aaa6d4a45 100644 --- a/core/src/main/java/google/registry/persistence/VKey.java +++ b/core/src/main/java/google/registry/persistence/VKey.java @@ -144,7 +144,7 @@ public class VKey extends ImmutableObject implements Serializable { */ public static VKey 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 kvs = @@ -307,6 +307,7 @@ public class VKey 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(); } diff --git a/core/src/main/java/google/registry/tools/GetEppResourceCommand.java b/core/src/main/java/google/registry/tools/GetEppResourceCommand.java index 6b89518c4..3a55badf5 100644 --- a/core/src/main/java/google/registry/tools/GetEppResourceCommand.java +++ b/core/src/main/java/google/registry/tools/GetEppResourceCommand.java @@ -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)); } diff --git a/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java b/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java index 5d42e24a9..65281a438 100644 --- a/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java +++ b/core/src/test/java/google/registry/batch/AsyncTaskEnqueuerTest.java @@ -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( diff --git a/core/src/test/java/google/registry/batch/DeleteContactsAndHostsActionTest.java b/core/src/test/java/google/registry/batch/DeleteContactsAndHostsActionTest.java index df5a863d9..47fcec74a 100644 --- a/core/src/test/java/google/registry/batch/DeleteContactsAndHostsActionTest.java +++ b/core/src/test/java/google/registry/batch/DeleteContactsAndHostsActionTest.java @@ -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") diff --git a/core/src/test/java/google/registry/batch/RefreshDnsOnHostRenameActionTest.java b/core/src/test/java/google/registry/batch/RefreshDnsOnHostRenameActionTest.java index 842c6b9be..9196b35e9 100644 --- a/core/src/test/java/google/registry/batch/RefreshDnsOnHostRenameActionTest.java +++ b/core/src/test/java/google/registry/batch/RefreshDnsOnHostRenameActionTest.java @@ -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(); } diff --git a/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java b/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java index b8b3831f4..f77cdb86e 100644 --- a/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java +++ b/core/src/test/java/google/registry/batch/ResaveEntityActionTest.java @@ -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)), diff --git a/core/src/test/java/google/registry/flows/ResourceFlowTestCase.java b/core/src/test/java/google/registry/flows/ResourceFlowTestCase.java index a149b7cd0..44d628641 100644 --- a/core/src/test/java/google/registry/flows/ResourceFlowTestCase.java +++ b/core/src/test/java/google/registry/flows/ResourceFlowTestCase.java @@ -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 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 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")) diff --git a/core/src/test/java/google/registry/tools/GetContactCommandTest.java b/core/src/test/java/google/registry/tools/GetContactCommandTest.java index 236429f6d..c345e5d8b 100644 --- a/core/src/test/java/google/registry/tools/GetContactCommandTest.java +++ b/core/src/test/java/google/registry/tools/GetContactCommandTest.java @@ -42,7 +42,11 @@ class GetContactCommandTest extends CommandTestCase { 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 { 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 { 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 diff --git a/core/src/test/java/google/registry/tools/GetDomainCommandTest.java b/core/src/test/java/google/registry/tools/GetDomainCommandTest.java index 9a38bd139..ddeb21421 100644 --- a/core/src/test/java/google/registry/tools/GetDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/GetDomainCommandTest.java @@ -43,7 +43,11 @@ class GetDomainCommandTest extends CommandTestCase { 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 { 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 { 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 diff --git a/core/src/test/java/google/registry/tools/GetHostCommandTest.java b/core/src/test/java/google/registry/tools/GetHostCommandTest.java index 33bfc8079..d4fecee1d 100644 --- a/core/src/test/java/google/registry/tools/GetHostCommandTest.java +++ b/core/src/test/java/google/registry/tools/GetHostCommandTest.java @@ -42,7 +42,11 @@ class GetHostCommandTest extends CommandTestCase { 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 { 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 { 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 diff --git a/core/src/test/java/google/registry/tools/ResaveEntitiesCommandTest.java b/core/src/test/java/google/registry/tools/ResaveEntitiesCommandTest.java index e09e14fbd..0e572eddf 100644 --- a/core/src/test/java/google/registry/tools/ResaveEntitiesCommandTest.java +++ b/core/src/test/java/google/registry/tools/ResaveEntitiesCommandTest.java @@ -52,6 +52,26 @@ class ResaveEntitiesCommandTest extends CommandTestCase { .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 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... types) { for (Class type : types) {