This is an automated email from the ASF dual-hosted git repository.
bamaer 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 ea4872e39f Look up message bundles once instead of up to four times,
fixes #9146 (#9147)
ea4872e39f is described below
commit ea4872e39f4ce3cb1e4be466c5b5994a3487ab15
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Fri Oct 30 06:15:29 2026 +0100
Look up message bundles once instead of up to four times, fixes #9146
(#9147)
* Look up message bundles once instead of up to four times, fixes #9146
* Remember which message bundles the system class loader has
---
.../apache/hop/i18n/ClassPathPropertiesIndex.java | 127 +++++++++++++++++++++
.../org/apache/hop/i18n/GlobalMessageUtil.java | 78 ++++++++++++-
.../hop/i18n/ClassPathPropertiesIndexTest.java | 93 +++++++++++++++
.../org/apache/hop/i18n/GlobalMessageUtilTest.java | 56 +++++++++
4 files changed, 351 insertions(+), 3 deletions(-)
diff --git
a/core/src/main/java/org/apache/hop/i18n/ClassPathPropertiesIndex.java
b/core/src/main/java/org/apache/hop/i18n/ClassPathPropertiesIndex.java
new file mode 100644
index 0000000000..ddf11ab718
--- /dev/null
+++ b/core/src/main/java/org/apache/hop/i18n/ClassPathPropertiesIndex.java
@@ -0,0 +1,127 @@
+/*
+ * 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.i18n;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.util.ArrayDeque;
+import java.util.Deque;
+import java.util.Enumeration;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.jar.Attributes;
+import java.util.jar.JarFile;
+import java.util.jar.Manifest;
+import java.util.zip.ZipEntry;
+
+/**
+ * The properties files on the system class path. Most candidate message
bundles exist nowhere, and
+ * looking one up searches every jar on the class path; with this list such a
lookup ends at once.
+ * The list is read from the jars at startup, in the background, so it always
matches what is on
+ * disk. Until it is ready, and when the class path holds something it can not
list, every lookup
+ * searches the class path as before.
+ */
+final class ClassPathPropertiesIndex {
+
+ private static volatile Set<String> names;
+
+ static {
+ Thread thread =
+ new Thread(
+ () ->
+ names =
+ read(
+ List.of(
+ System.getProperty("java.class.path",
"").split(File.pathSeparator))),
+ "Hop class path properties index");
+ thread.setDaemon(true);
+ thread.start();
+ }
+
+ private ClassPathPropertiesIndex() {}
+
+ /**
+ * @param resourceName a properties file, as a resource name
+ * @return true if the file is known not to be on the system class path,
false if it is there or
+ * the list is not available
+ */
+ static boolean isAbsent(String resourceName) {
+ Set<String> known = names;
+ return known != null && !known.contains(resourceName);
+ }
+
+ /**
+ * Lists the properties files in the given class path entries and in the
jars their manifests add,
+ * as the system class loader sees them.
+ *
+ * @return the resource names, or null if an entry can not be listed
+ */
+ static Set<String> read(List<String> classPath) {
+ Set<String> found = new HashSet<>();
+ Set<File> seen = new HashSet<>();
+ Deque<File> todo = new ArrayDeque<>();
+ for (String entry : classPath) {
+ if (!entry.isEmpty()) {
+ todo.add(new File(entry).getAbsoluteFile());
+ }
+ }
+ while (!todo.isEmpty()) {
+ File file = todo.poll();
+ if (!seen.add(file) || !file.exists()) {
+ // The class loader skips a missing entry as well
+ continue;
+ }
+ if (!file.isFile()) {
+ // A folder (a development setup): not listed, so no lookup is skipped
+ return null;
+ }
+ try (JarFile jar = new JarFile(file, false)) {
+ Enumeration<? extends ZipEntry> entries = jar.entries();
+ while (entries.hasMoreElements()) {
+ String name = entries.nextElement().getName();
+ if (name.endsWith(".properties")) {
+ found.add(name);
+ }
+ }
+ Manifest manifest = jar.getManifest();
+ String extra =
+ manifest == null
+ ? null
+ :
manifest.getMainAttributes().getValue(Attributes.Name.CLASS_PATH);
+ if (extra != null) {
+ for (String url : extra.trim().split("\\s+")) {
+ if (url.isEmpty()) {
+ continue;
+ }
+ // URLs relative to the jar, as the class loader reads them
+ URI uri = file.getParentFile().toURI().resolve(url);
+ if (!"file".equals(uri.getScheme())) {
+ return null;
+ }
+ todo.add(new File(uri).getAbsoluteFile());
+ }
+ }
+ } catch (IOException | IllegalArgumentException e) {
+ return null;
+ }
+ }
+ return found;
+ }
+}
diff --git a/core/src/main/java/org/apache/hop/i18n/GlobalMessageUtil.java
b/core/src/main/java/org/apache/hop/i18n/GlobalMessageUtil.java
index fe1451b2f4..8f9fecdca8 100644
--- a/core/src/main/java/org/apache/hop/i18n/GlobalMessageUtil.java
+++ b/core/src/main/java/org/apache/hop/i18n/GlobalMessageUtil.java
@@ -28,13 +28,17 @@ import java.text.MessageFormat;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Locale;
+import java.util.Map;
import java.util.MissingResourceException;
+import java.util.Optional;
import java.util.PropertyResourceBundle;
import java.util.ResourceBundle;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import org.apache.commons.lang3.StringUtils;
import org.apache.hop.core.Const;
import org.apache.hop.core.exception.HopException;
+import org.apache.hop.core.plugins.HopURLClassLoader;
public class GlobalMessageUtil {
@@ -462,6 +466,22 @@ public class GlobalMessageUtil {
*/
static class GlobalMessageControl extends ResourceBundle.Control {
+ private static final Map<String, Optional<URL>> SYSTEM_RESOURCES = new
ConcurrentHashMap<>();
+
+ /** Whether a class loader class still finds resources the way
HopURLClassLoader does. */
+ private static final ClassValue<Boolean> USES_HOP_RESOURCE_ORDER =
+ new ClassValue<>() {
+ @Override
+ protected Boolean computeValue(final Class<?> type) {
+ try {
+ return type.getMethod("getResource",
String.class).getDeclaringClass()
+ == HopURLClassLoader.class;
+ } catch (NoSuchMethodException e) {
+ return false;
+ }
+ }
+ };
+
private boolean fallbackOnRoot;
GlobalMessageControl(final boolean fallbackOnRoot) {
@@ -485,6 +505,12 @@ public class GlobalMessageUtil {
return locales;
}
+ @Override
+ public List<String> getFormats(final String baseName) {
+ // newBundle() only reads properties files: also asking for java.class
repeats every lookup
+ return FORMAT_PROPERTIES;
+ }
+
@Override
public ResourceBundle newBundle(
final String baseName,
@@ -506,11 +532,12 @@ public class GlobalMessageUtil {
}
}
} else {
- stream = loader.getResourceAsStream(resourceName);
+ stream = openStream(findResource(loader, resourceName));
}
- if (stream == null) {
+ if (stream == null && !delegatesToSystemClassLoader(loader)) {
// Retry with the system class loader, just in case we are dealing
with a messy plug-in.
- stream = ClassLoader.getSystemResourceAsStream(resourceName);
+ // A loader that delegates to the system class loader has already
searched it.
+ stream = openStream(findSystemResource(resourceName));
}
if (stream != null) {
try {
@@ -527,5 +554,50 @@ public class GlobalMessageUtil {
}
return bundle;
}
+
+ /**
+ * Looks a bundle up the way the loader would, but answers the system
class loader's part from a
+ * cache. Most candidate bundles exist nowhere, and every plugin loader
would otherwise search
+ * all the jars on the class path again for each of them.
+ */
+ private static URL findResource(final ClassLoader loader, final String
name) {
+ if (loader == ClassLoader.getSystemClassLoader()) {
+ return findSystemResource(name);
+ }
+ if (loader instanceof HopURLClassLoader hopLoader
+ && USES_HOP_RESOURCE_ORDER.get(loader.getClass())) {
+ // HopURLClassLoader.getResource(): this loader's own jars first, then
the parent
+ final URL url = hopLoader.findResource(name);
+ if (url != null || hopLoader.getParent() == null) {
+ return url;
+ }
+ return findResource(hopLoader.getParent(), name);
+ }
+ return loader.getResource(name);
+ }
+
+ private static URL findSystemResource(final String name) {
+ if (ClassPathPropertiesIndex.isAbsent(name)) {
+ return null;
+ }
+ // The system class path does not change while Hop runs
+ return SYSTEM_RESOURCES
+ .computeIfAbsent(name, n ->
Optional.ofNullable(ClassLoader.getSystemResource(n)))
+ .orElse(null);
+ }
+
+ private static InputStream openStream(final URL url) throws IOException {
+ return url == null ? null : url.openStream();
+ }
+
+ static boolean delegatesToSystemClassLoader(final ClassLoader loader) {
+ final ClassLoader system = ClassLoader.getSystemClassLoader();
+ for (ClassLoader current = loader; current != null; current =
current.getParent()) {
+ if (current == system) {
+ return true;
+ }
+ }
+ return false;
+ }
}
}
diff --git
a/core/src/test/java/org/apache/hop/i18n/ClassPathPropertiesIndexTest.java
b/core/src/test/java/org/apache/hop/i18n/ClassPathPropertiesIndexTest.java
new file mode 100644
index 0000000000..9214aa721b
--- /dev/null
+++ b/core/src/test/java/org/apache/hop/i18n/ClassPathPropertiesIndexTest.java
@@ -0,0 +1,93 @@
+/*
+ * 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.i18n;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Set;
+import java.util.jar.Attributes;
+import java.util.jar.JarEntry;
+import java.util.jar.JarOutputStream;
+import java.util.jar.Manifest;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+class ClassPathPropertiesIndexTest {
+
+ @TempDir Path folder;
+
+ private Path jar(String name, String classPath, String... entries) throws
IOException {
+ Manifest manifest = new Manifest();
+ manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
+ if (classPath != null) {
+ manifest.getMainAttributes().put(Attributes.Name.CLASS_PATH, classPath);
+ }
+ Path jar = folder.resolve(name);
+ try (OutputStream out = Files.newOutputStream(jar);
+ JarOutputStream jarOut = new JarOutputStream(out, manifest)) {
+ for (String entry : entries) {
+ jarOut.putNextEntry(new JarEntry(entry));
+ jarOut.closeEntry();
+ }
+ }
+ return jar;
+ }
+
+ @Test
+ void listsThePropertiesFilesOfTheJars() throws Exception {
+ Path first = jar("first.jar", null,
"a/messages/messages_en_US.properties", "a/A.class");
+ Path second = jar("second.jar", null,
"b/messages/messages_fr_FR.properties");
+ assertEquals(
+ Set.of("a/messages/messages_en_US.properties",
"b/messages/messages_fr_FR.properties"),
+ ClassPathPropertiesIndex.read(List.of(first.toString(),
second.toString())));
+ }
+
+ @Test
+ void followsManifestClassPathEntries() throws Exception {
+ Files.createDirectories(folder.resolve("lib dir"));
+ jar("lib dir/extra.jar", null, "c/messages/messages_en_US.properties");
+ Path main = jar("main.jar", "lib%20dir/extra.jar missing.jar");
+ assertEquals(
+ Set.of("c/messages/messages_en_US.properties"),
+ ClassPathPropertiesIndex.read(List.of(main.toString())));
+ }
+
+ @Test
+ void skipsMissingEntries() throws Exception {
+ Path first = jar("first.jar", null,
"a/messages/messages_en_US.properties");
+ assertEquals(
+ Set.of("a/messages/messages_en_US.properties"),
+ ClassPathPropertiesIndex.read(
+ List.of(folder.resolve("nothing.jar").toString(),
first.toString())));
+ }
+
+ @Test
+ void aFolderOnTheClassPathTurnsTheListOff() throws Exception {
+ Path first = jar("first.jar", null,
"a/messages/messages_en_US.properties");
+ Files.createDirectories(folder.resolve("classes"));
+ assertNull(
+ ClassPathPropertiesIndex.read(
+ List.of(first.toString(), folder.resolve("classes").toString())));
+ }
+}
diff --git a/core/src/test/java/org/apache/hop/i18n/GlobalMessageUtilTest.java
b/core/src/test/java/org/apache/hop/i18n/GlobalMessageUtilTest.java
index 97a51953d3..3c38655bc8 100644
--- a/core/src/test/java/org/apache/hop/i18n/GlobalMessageUtilTest.java
+++ b/core/src/test/java/org/apache/hop/i18n/GlobalMessageUtilTest.java
@@ -21,7 +21,10 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.net.URL;
+import java.net.URLClassLoader;
import java.util.Locale;
+import java.util.ResourceBundle;
import org.apache.hop.junit.rules.RestoreHopEnvironmentExtension;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@@ -55,6 +58,59 @@ class GlobalMessageUtilTest {
assertFalse(GlobalMessageUtil.isMissingKey(" !"));
}
+ @Test
+ void bundlesAreOnlyLookedUpAsPropertiesFiles() {
+ assertEquals(
+ ResourceBundle.Control.FORMAT_PROPERTIES,
+ new
GlobalMessageUtil.GlobalMessageControl(true).getFormats("any.base.name"));
+ }
+
+ @Test
+ void bundleIsFoundThroughALoaderThatDoesNotDelegateToTheSystemLoader()
throws Exception {
+ // A loader without the system class loader as an ancestor: the bundle
comes from the retry
+ // on the system class loader
+ ClassLoader isolated = new URLClassLoader(new URL[0],
ClassLoader.getPlatformClassLoader());
+ ResourceBundle bundle =
+ new GlobalMessageUtil.GlobalMessageControl(true)
+ .newBundle(
+ GlobalMessages.SYSTEM_BUNDLE_PACKAGE + "." +
GlobalMessages.BUNDLE_NAME,
+ Locale.FRANCE,
+ "java.properties",
+ isolated,
+ false);
+ assertEquals("Une certaine valeur {0}", bundle.getString("someKey"));
+ }
+
+ @Test
+ void noRetryOnTheSystemLoaderForALoaderThatDelegatesToIt() {
+ ClassLoader system = ClassLoader.getSystemClassLoader();
+
assertTrue(GlobalMessageUtil.GlobalMessageControl.delegatesToSystemClassLoader(system));
+ assertTrue(
+ GlobalMessageUtil.GlobalMessageControl.delegatesToSystemClassLoader(
+ new URLClassLoader(new URL[0], system)));
+ assertTrue(
+ GlobalMessageUtil.GlobalMessageControl.delegatesToSystemClassLoader(
+ new URLClassLoader(new URL[0], new URLClassLoader(new URL[0],
system))));
+ assertFalse(
+ GlobalMessageUtil.GlobalMessageControl.delegatesToSystemClassLoader(
+ new URLClassLoader(new URL[0],
ClassLoader.getPlatformClassLoader())));
+ }
+
+ @Test
+ void bundleIsFoundThroughALoaderThatDelegatesToTheSystemLoader() throws
Exception {
+ // No retry for this loader: the bundle comes from its own parent chain
+ ClassLoader child = new URLClassLoader(new URL[0],
ClassLoader.getSystemClassLoader());
+ ResourceBundle bundle =
+ new GlobalMessageUtil.GlobalMessageControl(true)
+ .newBundle(
+ GlobalMessages.SYSTEM_BUNDLE_PACKAGE + "." +
GlobalMessages.BUNDLE_NAME,
+ Locale.FRANCE,
+ "java.properties",
+ child,
+ false);
+ assertEquals("Une certaine valeur {0}", bundle.getString("someKey"));
+ }
+
@Test
void calculateString() {
