[HUDI-3730] Improve meta sync class design and hierarchies (#5854)

* [HUDI-3730] Improve meta sync class design and hierarchies (#5754)
* Implements class design proposed in RFC-55

Co-authored-by: jian.feng <fengjian428@gmial.com>
Co-authored-by: jian.feng <jian.feng@shopee.com>

hudi-sync/hudi-sync-common/src/test/java/org/apache/hudi/sync/common/util/TestSyncUtilHelpers.java

@@ -20,16 +20,19 @@ package org.apache.hudi.sync.common.util;
 import org.apache.hudi.common.config.TypedProperties;
 import org.apache.hudi.common.fs.FSUtils;
 import org.apache.hudi.exception.HoodieException;
-import org.apache.hudi.sync.common.AbstractSyncTool;
+import org.apache.hudi.sync.common.HoodieSyncTool;
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import java.io.IOException;
 import java.util.Properties;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -46,42 +49,44 @@ public class TestSyncUtilHelpers {
     hadoopConf = fileSystem.getConf();
   }
 
-  @Test
-  public void testCreateValidSyncClass() {
-    AbstractSyncTool metaSyncTool = SyncUtilHelpers.instantiateMetaSyncTool(
-        ValidMetaSyncClass.class.getName(),
+  @ParameterizedTest
+  @ValueSource(classes = {DummySyncTool1.class, DummySyncTool2.class})
+  public void testCreateValidSyncClass(Class<?> clazz) {
+    HoodieSyncTool syncTool = SyncUtilHelpers.instantiateMetaSyncTool(
+        clazz.getName(),
         new TypedProperties(),
         hadoopConf,
         fileSystem,
         BASE_PATH,
         BASE_FORMAT
     );
-    assertTrue(metaSyncTool instanceof ValidMetaSyncClass);
+    assertTrue(clazz.isAssignableFrom(syncTool.getClass()));
   }
 
   /**
-   * Ensure it still works for the deprecated constructor of {@link AbstractSyncTool}
+   * Ensure it still works for the deprecated constructor of {@link HoodieSyncTool}
    * as we implemented the fallback.
    */
-  @Test
-  public void testCreateDeprecatedSyncClass() {
+  @ParameterizedTest
+  @ValueSource(classes = {DeprecatedSyncTool1.class, DeprecatedSyncTool2.class})
+  public void testCreateDeprecatedSyncClass(Class<?> clazz) {
     Properties properties = new Properties();
-    AbstractSyncTool deprecatedMetaSyncClass = SyncUtilHelpers.instantiateMetaSyncTool(
-        DeprecatedMetaSyncClass.class.getName(),
+    HoodieSyncTool syncTool = SyncUtilHelpers.instantiateMetaSyncTool(
+        clazz.getName(),
         new TypedProperties(properties),
         hadoopConf,
         fileSystem,
         BASE_PATH,
         BASE_FORMAT
     );
-    assertTrue(deprecatedMetaSyncClass instanceof DeprecatedMetaSyncClass);
+    assertTrue(clazz.isAssignableFrom(syncTool.getClass()));
   }
 
   @Test
   public void testCreateInvalidSyncClass() {
-    Exception exception = assertThrows(HoodieException.class, () -> {
+    Throwable t = assertThrows(HoodieException.class, () -> {
       SyncUtilHelpers.instantiateMetaSyncTool(
-          InvalidSyncClass.class.getName(),
+          InvalidSyncTool.class.getName(),
           new TypedProperties(),
           hadoopConf,
           fileSystem,
@@ -90,14 +95,14 @@ public class TestSyncUtilHelpers {
       );
     });
 
-    String expectedMessage = "Could not load meta sync class " + InvalidSyncClass.class.getName();
-    assertTrue(exception.getMessage().contains(expectedMessage));
-
+    String expectedMessage = "Could not load meta sync class " + InvalidSyncTool.class.getName()
+        + ": no valid constructor found.";
+    assertEquals(expectedMessage, t.getMessage());
   }
 
-  public static class ValidMetaSyncClass extends AbstractSyncTool {
-    public ValidMetaSyncClass(TypedProperties props, Configuration conf, FileSystem fs) {
-      super(props, conf, fs);
+  public static class DummySyncTool1 extends HoodieSyncTool {
+    public DummySyncTool1(Properties props, Configuration hadoopConf) {
+      super(props, hadoopConf);
     }
 
     @Override
@@ -106,9 +111,9 @@ public class TestSyncUtilHelpers {
     }
   }
 
-  public static class DeprecatedMetaSyncClass extends AbstractSyncTool {
-    public DeprecatedMetaSyncClass(Properties props, FileSystem fileSystem) {
-      super(props, fileSystem);
+  public static class DummySyncTool2 extends HoodieSyncTool {
+    public DummySyncTool2(Properties props, Configuration hadoopConf) {
+      super(props, hadoopConf);
     }
 
     @Override
@@ -117,8 +122,30 @@ public class TestSyncUtilHelpers {
     }
   }
 
-  public static class InvalidSyncClass {
-    public InvalidSyncClass(Properties props) {
+  public static class DeprecatedSyncTool1 extends HoodieSyncTool {
+    public DeprecatedSyncTool1(TypedProperties props, Configuration hadoopConf, FileSystem fs) {
+      super(props, hadoopConf, fs);
+    }
+
+    @Override
+    public void syncHoodieTable() {
+      throw new HoodieException("Method unimplemented as its a test class");
+    }
+  }
+
+  public static class DeprecatedSyncTool2 extends HoodieSyncTool {
+    public DeprecatedSyncTool2(Properties props, FileSystem fs) {
+      super(props, fs);
+    }
+
+    @Override
+    public void syncHoodieTable() {
+      throw new HoodieException("Method unimplemented as its a test class");
+    }
+  }
+
+  public static class InvalidSyncTool {
+    public InvalidSyncTool(Properties props, FileSystem fs, Configuration hadoopConf) {
     }
   }
 }