[GitHub] [incubator-seatunnel] ruanwenjun commented on a diff in pull request #2193: [Core][Starter] Change jar connector load logic

[GitBox <gi...@apache.org>]

ruanwenjun commented on code in PR #2193:
URL: https://github.com/apache/incubator-seatunnel/pull/2193#discussion_r923058604


##########
seatunnel-plugin-discovery/src/main/java/org/apache/seatunnel/plugin/discovery/AbstractPluginDiscovery.java:
##########
@@ -40,15 +43,34 @@
 import java.util.Optional;
 import java.util.ServiceLoader;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.BiConsumer;
 import java.util.stream.Collectors;
 
 public abstract class AbstractPluginDiscovery<T> implements PluginDiscovery<T> {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(AbstractPluginDiscovery.class);
     private final Path pluginDir;
 
+    /**
+     * Add jar url to classloader. The different engine should have different logic to add url into
+     * their own classloader
+     */
+    private BiConsumer<ClassLoader, URL> addURLToClassLoader = (classLoader, url) -> {
+        if (classLoader instanceof URLClassLoader) {
+            ReflectionUtils.invoke(classLoader, "addURL", url);
+        } else {
+            throw new UnsupportedOperationException("can't support custom load jar");
+        }
+    };
+
     protected final ConcurrentHashMap<PluginIdentifier, Optional<URL>> pluginJarPath =
-        new ConcurrentHashMap<>(Common.COLLECTION_SIZE);
+            new ConcurrentHashMap<>(Common.COLLECTION_SIZE);
+
+    public AbstractPluginDiscovery(String pluginSubDir, BiConsumer<ClassLoader, URL> addURLToClassloader) {

Review Comment:
   It's better to move the addURL logic to this class, and the other class doesn't need to submit this parameter to tell the AbstractPluginDiscovery how to add URL, since we don't expose the classloader to them, so they don't know which classloader we will use.
   ```suggestion
       public AbstractPluginDiscovery(String pluginSubDir) {
   ```



##########
seatunnel-core/seatunnel-flink-starter/src/main/java/org/apache/seatunnel/core/starter/flink/execution/AbstractPluginExecuteProcessor.java:
##########
@@ -41,6 +45,17 @@
     protected static final String ENGINE_TYPE = "seatunnel";
     protected static final String PLUGIN_NAME = "plugin_name";
 
+    protected final BiConsumer<ClassLoader, URL> addUrlToClassloader = (classLoader, url) -> {
+        if (classLoader.getClass().getName().endsWith("SafetyNetWrapperClassLoader")) {
+            URLClassLoader c = (URLClassLoader) ReflectionUtils.getField(classLoader, "inner").get();
+            ReflectionUtils.invoke(c, "addURL", url);

Review Comment:
   Why not directly use c.addURL
   ```suggestion
               c.addURL(url);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org