From 3327c1ae9b069933c883380cebfda39ffc54aab6 Mon Sep 17 00:00:00 2001 From: Michael Muller Date: Tue, 9 Mar 2021 07:12:15 -0500 Subject: [PATCH] Add a "ReplaySpecializer" to fix certain replays (#989) * Add a "ReplaySpecializer" to fix certain replays Due to the fact that a given entity in either database type can map to multiple entities in the other database, there are certain replication scenarios that don't quite work. Current known examples include: - propagation of cascading deletes from datastore to SQL - creation of datastore indexed entities for SQL entities (where indexes are a first-class concept) This change introduces a ReplaySpecializer class, which allows us to declare static method hooks at the entity class level that define any special operations that need to be performed before or after replaying a mutation for any given entity type. Currently, "before SQL delete" is the only supported hook. A change to DomainContent demonstrating how this facility can be used to fix problems in cascading delete propagation will be sent as a subsequent PR. * Throw exception on beforeSqlDelete failures * Changes for review --- .../backup/ReplayCommitLogsToSqlAction.java | 2 + .../registry/model/ofy/ReplayQueue.java | 5 +- .../schema/replay/ReplaySpecializer.java | 50 +++++++++++++++++++ .../ReplayCommitLogsToSqlActionTest.java | 16 ++++++ .../google/registry/testing/TestObject.java | 6 +++ 5 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 core/src/main/java/google/registry/schema/replay/ReplaySpecializer.java diff --git a/core/src/main/java/google/registry/backup/ReplayCommitLogsToSqlAction.java b/core/src/main/java/google/registry/backup/ReplayCommitLogsToSqlAction.java index cdc7feccf..978230c3f 100644 --- a/core/src/main/java/google/registry/backup/ReplayCommitLogsToSqlAction.java +++ b/core/src/main/java/google/registry/backup/ReplayCommitLogsToSqlAction.java @@ -37,6 +37,7 @@ import google.registry.request.auth.Auth; import google.registry.schema.replay.DatastoreEntity; import google.registry.schema.replay.DatastoreOnlyEntity; import google.registry.schema.replay.NonReplicatedEntity; +import google.registry.schema.replay.ReplaySpecializer; import google.registry.schema.replay.SqlReplayCheckpoint; import google.registry.util.RequestStatusChecker; import java.io.IOException; @@ -179,6 +180,7 @@ public class ReplayCommitLogsToSqlAction implements Runnable { if (!NonReplicatedEntity.class.isAssignableFrom(entityClass) && !DatastoreOnlyEntity.class.isAssignableFrom(entityClass) && entityClass.getAnnotation(javax.persistence.Entity.class) != null) { + ReplaySpecializer.beforeSqlDelete(entityVKey); jpaTm().delete(entityVKey); } } diff --git a/core/src/main/java/google/registry/model/ofy/ReplayQueue.java b/core/src/main/java/google/registry/model/ofy/ReplayQueue.java index a488daf18..96473f46b 100644 --- a/core/src/main/java/google/registry/model/ofy/ReplayQueue.java +++ b/core/src/main/java/google/registry/model/ofy/ReplayQueue.java @@ -24,6 +24,7 @@ import google.registry.config.RegistryEnvironment; import google.registry.model.UpdateAutoTimestamp; import google.registry.persistence.VKey; import google.registry.schema.replay.DatastoreEntity; +import google.registry.schema.replay.ReplaySpecializer; import java.util.HashMap; import java.util.Map; import java.util.concurrent.ConcurrentLinkedQueue; @@ -100,7 +101,9 @@ public class ReplayQueue { .forEach( entry -> { if (entry.getValue().equals(TransactionInfo.Delete.SENTINEL)) { - jpaTm().delete(VKey.from(entry.getKey())); + VKey vkey = VKey.from(entry.getKey()); + ReplaySpecializer.beforeSqlDelete(vkey); + jpaTm().delete(vkey); } else { ((DatastoreEntity) entry.getValue()) .toSqlEntity() diff --git a/core/src/main/java/google/registry/schema/replay/ReplaySpecializer.java b/core/src/main/java/google/registry/schema/replay/ReplaySpecializer.java new file mode 100644 index 000000000..d0e800bfa --- /dev/null +++ b/core/src/main/java/google/registry/schema/replay/ReplaySpecializer.java @@ -0,0 +1,50 @@ +// Copyright 2021 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.schema.replay; + +import com.google.common.flogger.FluentLogger; +import google.registry.persistence.VKey; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; + +/** + * Applies class-specific functions for model objects during transaction replays. + * + *

There are certain cases where changes to an entity require changes to other entities that are + * not directly present in the other database. This class allows us to do that by using reflection + * to invoke special class methods if they are present. + */ +public class ReplaySpecializer { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + + public static void beforeSqlDelete(VKey key) { + try { + Method method = key.getKind().getMethod("beforeSqlDelete", VKey.class); + method.invoke(null, new Object[] {key}); + } catch (NoSuchMethodException e) { + // Ignore, this just means that the class doesn't need this hook. + } catch (IllegalAccessException e) { + throw new RuntimeException( + "beforeSqlDelete() method is defined for class " + + key.getKind().getName() + + " but is not public.", + e); + } catch (InvocationTargetException e) { + throw new RuntimeException( + "beforeSqlDelete() method for class " + key.getKind().getName() + " threw an exception.", + e); + } + } +} diff --git a/core/src/test/java/google/registry/backup/ReplayCommitLogsToSqlActionTest.java b/core/src/test/java/google/registry/backup/ReplayCommitLogsToSqlActionTest.java index 7247ce3a1..b40d4e58b 100644 --- a/core/src/test/java/google/registry/backup/ReplayCommitLogsToSqlActionTest.java +++ b/core/src/test/java/google/registry/backup/ReplayCommitLogsToSqlActionTest.java @@ -126,6 +126,7 @@ public class ReplayCommitLogsToSqlActionTest { action.diffLister.gcsBucket = GCS_BUCKET; action.diffLister.executor = newDirectExecutorService(); RegistryConfig.overrideCloudSqlReplayCommitLogs(true); + TestObject.beforeSqlDeleteCallCount = 0; } @Test @@ -441,6 +442,21 @@ public class ReplayCommitLogsToSqlActionTest { .isEqualTo("Can't acquire SQL commit log replay lock, aborting."); } + @Test + void testSuccess_deleteSqlCallback() throws Exception { + DateTime now = fakeClock.nowUtc(); + jpaTm().transact(() -> SqlReplayCheckpoint.set(now.minusMinutes(1).minusMillis(1))); + saveDiffFile( + gcsService, + createCheckpoint(now.minusMinutes(1)), + CommitLogManifest.create( + getBucketKey(1), + now.minusMinutes(1), + ImmutableSet.of(Key.create(TestObject.create("to delete"))))); + action.run(); + assertThat(TestObject.beforeSqlDeleteCallCount).isEqualTo(1); + } + private void runAndAssertSuccess(DateTime expectedCheckpointTime) { action.run(); assertThat(response.getStatus()).isEqualTo(SC_OK); diff --git a/core/src/test/java/google/registry/testing/TestObject.java b/core/src/test/java/google/registry/testing/TestObject.java index c0658105e..61ba36e08 100644 --- a/core/src/test/java/google/registry/testing/TestObject.java +++ b/core/src/test/java/google/registry/testing/TestObject.java @@ -34,6 +34,8 @@ import javax.persistence.Transient; @EntityForTesting public class TestObject extends ImmutableObject implements DatastoreAndSqlEntity { + public static int beforeSqlDeleteCallCount; + @Parent @Transient Key parent; @Id @javax.persistence.Id String id; @@ -68,6 +70,10 @@ public class TestObject extends ImmutableObject implements DatastoreAndSqlEntity return instance; } + public static void beforeSqlDelete(VKey key) { + beforeSqlDeleteCallCount++; + } + /** A test @VirtualEntity model object, which should not be persisted. */ @Entity @VirtualEntity