This is an automated email from the ASF dual-hosted git repository.

mattcasters pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git


The following commit(s) were added to refs/heads/main by this push:
     new 989b1bf34e Issue #8755 : Keep extension points registered after the 
Hop environment is reset (#8773)
989b1bf34e is described below

commit 989b1bf34e7f43ade58acacea6b98b56b7ff8e29
Author: Matt Casters <[email protected]>
AuthorDate: Tue Oct 6 21:23:54 2026 +0200

    Issue #8755 : Keep extension points registered after the Hop environment is 
reset (#8773)
    
    PluginRegistry.reset() dropped the ExtensionPointMap listener. The UI-test 
harness resets the environment in the same JVM, so a pre-commit listener 
registered afterwards was never called and the nightly build failed in 
hop-misc-git.
---
 .../org/apache/hop/core/HopClientEnvironment.java  |  6 ++
 .../hop/core/extension/ExtensionPointMap.java      | 69 +++++++---------
 .../HopEnvironmentExtensionPointResetTest.java     | 91 ++++++++++++++++++++++
 3 files changed, 126 insertions(+), 40 deletions(-)

diff --git a/core/src/main/java/org/apache/hop/core/HopClientEnvironment.java 
b/core/src/main/java/org/apache/hop/core/HopClientEnvironment.java
index 0692b65a5c..f15fa79897 100644
--- a/core/src/main/java/org/apache/hop/core/HopClientEnvironment.java
+++ b/core/src/main/java/org/apache/hop/core/HopClientEnvironment.java
@@ -30,6 +30,7 @@ import org.apache.hop.core.encryption.Encr;
 import org.apache.hop.core.encryption.TwoWayPasswordEncoderPluginType;
 import org.apache.hop.core.exception.HopException;
 import org.apache.hop.core.exception.HopPluginException;
+import org.apache.hop.core.extension.ExtensionPointMap;
 import org.apache.hop.core.extension.ExtensionPointPluginType;
 import org.apache.hop.core.logging.ConsoleLoggingEventListener;
 import org.apache.hop.core.logging.HopLogStore;
@@ -231,6 +232,11 @@ public class HopClientEnvironment {
       HopLogStore.getInstance().reset();
     }
     PluginRegistry.getInstance().reset();
+    // reset() drops every plugin listener. The extension-point map subscribed 
once, when its class
+    // was loaded, and otherwise keeps calling plugins from the registry that 
was just discarded.
+    // Anything registered after the environment is rebuilt (a pre-commit 
check, for example) is
+    // then never invoked. Put the listener back and drop the stale entries.
+    ExtensionPointMap.getInstance().reset();
     initialized = null;
   }
 }
diff --git 
a/core/src/main/java/org/apache/hop/core/extension/ExtensionPointMap.java 
b/core/src/main/java/org/apache/hop/core/extension/ExtensionPointMap.java
index 4d27c1fc31..2dec4823c4 100644
--- a/core/src/main/java/org/apache/hop/core/extension/ExtensionPointMap.java
+++ b/core/src/main/java/org/apache/hop/core/extension/ExtensionPointMap.java
@@ -43,29 +43,35 @@ public class ExtensionPointMap {
 
   private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
 
-  private ExtensionPointMap(PluginRegistry pluginRegistry) {
-    this.registry = pluginRegistry;
-    extensionPointPluginMap = HashBasedTable.create();
-    registry.addPluginListener(
-        ExtensionPointPluginType.class,
-        new IPluginTypeListener() {
+  /**
+   * One listener for the life of this singleton. {@link 
PluginRegistry#reset()} drops every
+   * listener; {@link #reset()} puts this same instance back. A fresh listener 
on every reset would
+   * stack, and each one would be notified.
+   */
+  private final IPluginTypeListener pluginListener =
+      new IPluginTypeListener() {
 
-          @Override
-          public void pluginAdded(Object serviceObject) {
-            addExtensionPoint((IPlugin) serviceObject);
-          }
+        @Override
+        public void pluginAdded(Object serviceObject) {
+          addExtensionPoint((IPlugin) serviceObject);
+        }
 
-          @Override
-          public void pluginRemoved(Object serviceObject) {
-            removeExtensionPoint((IPlugin) serviceObject);
-          }
+        @Override
+        public void pluginRemoved(Object serviceObject) {
+          removeExtensionPoint((IPlugin) serviceObject);
+        }
 
-          @Override
-          public void pluginChanged(Object serviceObject) {
-            removeExtensionPoint((IPlugin) serviceObject);
-            addExtensionPoint((IPlugin) serviceObject);
-          }
-        });
+        @Override
+        public void pluginChanged(Object serviceObject) {
+          removeExtensionPoint((IPlugin) serviceObject);
+          addExtensionPoint((IPlugin) serviceObject);
+        }
+      };
+
+  private ExtensionPointMap(PluginRegistry pluginRegistry) {
+    this.registry = pluginRegistry;
+    extensionPointPluginMap = HashBasedTable.create();
+    registry.addPluginListener(ExtensionPointPluginType.class, pluginListener);
 
     List<IPlugin> extensionPointPlugins = 
registry.getPlugins(ExtensionPointPluginType.class);
     for (IPlugin extensionPointPlugin : extensionPointPlugins) {
@@ -242,26 +248,9 @@ public class ExtensionPointMap {
     lock.writeLock().lock();
     try {
       extensionPointPluginMap.clear();
-      registry.addPluginListener(
-          ExtensionPointPluginType.class,
-          new IPluginTypeListener() {
-
-            @Override
-            public void pluginAdded(Object serviceObject) {
-              addExtensionPoint((IPlugin) serviceObject);
-            }
-
-            @Override
-            public void pluginRemoved(Object serviceObject) {
-              removeExtensionPoint((IPlugin) serviceObject);
-            }
-
-            @Override
-            public void pluginChanged(Object serviceObject) {
-              removeExtensionPoint((IPlugin) serviceObject);
-              addExtensionPoint((IPlugin) serviceObject);
-            }
-          });
+      // HashSet.add of the same instance is a no-op when the listener is 
already registered, and
+      // puts it back after PluginRegistry.reset() has cleared the listener 
set.
+      registry.addPluginListener(ExtensionPointPluginType.class, 
pluginListener);
     } finally {
       lock.writeLock().unlock();
     }
diff --git 
a/engine/src/test/java/org/apache/hop/core/HopEnvironmentExtensionPointResetTest.java
 
b/engine/src/test/java/org/apache/hop/core/HopEnvironmentExtensionPointResetTest.java
new file mode 100644
index 0000000000..1f286ef49e
--- /dev/null
+++ 
b/engine/src/test/java/org/apache/hop/core/HopEnvironmentExtensionPointResetTest.java
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.hop.core;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.hop.core.extension.ExtensionPointHandler;
+import org.apache.hop.core.extension.ExtensionPointMap;
+import org.apache.hop.core.extension.ExtensionPointPluginType;
+import org.apache.hop.core.extension.IExtensionPoint;
+import org.apache.hop.core.logging.ILogChannel;
+import org.apache.hop.core.logging.LogChannel;
+import org.apache.hop.core.plugins.IPlugin;
+import org.apache.hop.core.plugins.PluginRegistry;
+import org.apache.hop.core.variables.IVariables;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * The UI-test harness calls {@link HopEnvironment#reset()} before {@link 
HopEnvironment#init()}.
+ * Loading a pipeline earlier in the same JVM constructs {@link 
ExtensionPointMap}, and {@code
+ * PluginRegistry.reset()} used to drop that map's listener. A plugin 
registered afterwards, such as
+ * the git pre-commit check, was then never called.
+ */
+class HopEnvironmentExtensionPointResetTest {
+
+  private static final String PLUGIN_ID = 
"HopEnvironmentExtensionPointResetTest";
+  private static final String EXTENSION_POINT_ID = 
"HopEnvironmentExtensionPointReset";
+
+  @AfterEach
+  void removeTestListener() {
+    PluginRegistry registry = PluginRegistry.getInstance();
+    IPlugin plugin = registry.getPlugin(ExtensionPointPluginType.class, 
PLUGIN_ID);
+    if (plugin != null) {
+      registry.removePlugin(ExtensionPointPluginType.class, plugin);
+    }
+    HopEnvironment.reset();
+  }
+
+  @Test
+  void extensionPointRegisteredAfterEnvironmentResetIsCalled() throws 
Exception {
+    // Construct the map first, the way an earlier test that loads a pipeline 
does.
+    ExtensionPointMap.getInstance();
+    RecordingExtension.called = false;
+
+    HopEnvironment.reset();
+    HopEnvironment.init();
+
+    ExtensionPointPluginType.getInstance()
+        .registerCustom(
+            RecordingExtension.class,
+            "test",
+            PLUGIN_ID,
+            EXTENSION_POINT_ID,
+            "Listener registered after the environment is reset",
+            null);
+
+    ExtensionPointHandler.callExtensionPoint(
+        LogChannel.GENERAL, null, EXTENSION_POINT_ID, "payload");
+
+    assertTrue(RecordingExtension.called);
+    assertFalse(RecordingExtension.calledWithNull);
+  }
+
+  public static class RecordingExtension implements IExtensionPoint<Object> {
+    static volatile boolean called;
+    static volatile boolean calledWithNull;
+
+    @Override
+    public void callExtensionPoint(ILogChannel log, IVariables variables, 
Object object) {
+      called = true;
+      calledWithNull = object == null;
+    }
+  }
+}

Reply via email to