Merge pull request #5095 from thomaslow/performance-loading-translations

Performance Loading Translations

Kitodo/src/main/java/org/kitodo/production/helper/messages/CustomResourceBundle.java

@@ -15,10 +15,14 @@
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLClassLoader;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.util.Enumeration;
+import java.util.HashMap;
 import java.util.Locale;
+import java.util.Map;
 import java.util.MissingResourceException;
 import java.util.Objects;
 import java.util.ResourceBundle;
@@ -32,6 +36,8 @@
 abstract class CustomResourceBundle extends ResourceBundle {
 
     private static final Logger logger = LogManager.getLogger(CustomResourceBundle.class);
+    private static URLClassLoader urlClassLoader;
+    private static Map<String, Boolean> propertiesFileExistsMap = new HashMap<String, Boolean>();
 
     @Override
     public Enumeration<String> getKeys() {
@@ -44,7 +50,92 @@ protected Object handleGetObject(String key) {
     }
 
     /**
-     * Get custom resource bundle. In case there is a custom version of translation
+     * Create a URLClassLoader that is allowed to load resource bundles from the external directory
+     * containing translated messages and errors (e.g. /usr/local/kitodo/messages).
+
+     * @return instance of URLClassLoader
+     */
+    private static URLClassLoader getURLClassLoader() {
+        if (Objects.isNull(urlClassLoader)) {
+            File file = new File(ConfigCore.getParameterOrDefaultValue(ParameterCore.DIR_LOCAL_MESSAGES));
+            if (file.exists()) {
+                try {
+                    final URL resourceURL = file.toURI().toURL();
+                    urlClassLoader = AccessController.doPrivileged(
+                        (PrivilegedAction<URLClassLoader>) () -> new URLClassLoader(new URL[] { resourceURL })
+                    );
+                } catch (MalformedURLException e) {
+                    logger.info(e.getMessage(), e);
+                }
+            } else {
+                urlClassLoader = null;
+            }
+        }
+        return urlClassLoader;
+    }
+
+    /**
+     * Checks if properties file for a specfiic external resource bundle does exist.
+     * Remembers existence in static map such that filesystem is not checked repeatedly.
+     * 
+     * <p>This check is required because resource bundles seem to always load if an URLClassLoader
+     * could be initialized even if the corresponding properties file in that directory does not 
+     * exist. The resulting resource bundle is simply empty then.</p>
+     * 
+     * <p>This check allows to only load from URLClassLoader, if the corresponding properties file
+     * actually exists.</p>
+     * 
+     * @param bundleName the bundle name
+     * @param locale the locale
+     * @return true if properties file for resource bundle exists, else false
+     */
+    private static Boolean externalResourceBundleFileExists(String bundleName, Locale locale) {
+        String key = bundleName + "_" + locale.getLanguage();
+        if (!propertiesFileExistsMap.containsKey(key)) {
+            String directory = ConfigCore.getParameterOrDefaultValue(ParameterCore.DIR_LOCAL_MESSAGES);
+            Path path = Paths.get(directory, bundleName + "_" + locale.getLanguage() + ".properties");
+            File file = path.toFile();
+
+            propertiesFileExistsMap.put(key, file.exists());
+
+            if (!file.exists()) {
+                logger.error("Could not find external resource bundle '" + bundleName + "' at " + file);
+            }
+        }
+        return propertiesFileExistsMap.get(key);
+    }
+
+    /**
+     * Loads an external resource bundle (outside jar files) if a corresponding properties file 
+     * exists and an URLClassLoader could be build that has the permissions to load files from 
+     * that directory.
+     * 
+     * @param bundleName the bundle name
+     * @param locale the locale
+     * @return the external resource bundle or null if it does not exist
+     */
+    private static ResourceBundle getExternalResourceBundle(String bundleName, Locale locale) {
+        if (!externalResourceBundleFileExists(bundleName, locale)) {
+            return null;
+        }
+        URLClassLoader urlLoader = getURLClassLoader();
+        if (Objects.nonNull(urlLoader)) {
+            try {
+                return ResourceBundle.getBundle(bundleName, locale, urlLoader);
+            } catch (MissingResourceException e) {
+                logger.error("Could not load external resource bundle '" + bundleName + "': " + e.getMessage());
+            }
+        }
+        return null;
+    }
+
+    private static ResourceBundle getExternalResourceBundle(String bundleName) {
+        Locale locale = LocaleHelper.getCurrentLocale();
+        return getExternalResourceBundle(bundleName, locale);
+    }
+
+    /**
+     * Get resource bundle. In case there is a custom version of translation
      * files load them, if not load the default ones.
      * 
      * @param defaultBundleName
@@ -56,43 +147,32 @@ protected Object handleGetObject(String key) {
      * @return available translation bundle
      */
     public static ResourceBundle getResourceBundle(String defaultBundleName, String customBundleName, Locale locale) {
-        File file = new File(ConfigCore.getParameterOrDefaultValue(ParameterCore.DIR_LOCAL_MESSAGES));
-        if (file.exists()) {
-            try {
-                final URL resourceURL = file.toURI().toURL();
-                URLClassLoader urlLoader = AccessController.doPrivileged(
-                    (PrivilegedAction<URLClassLoader>) () -> new URLClassLoader(new URL[] {resourceURL }));
-                return ResourceBundle.getBundle(customBundleName, locale, urlLoader);
-            } catch (MalformedURLException | MissingResourceException e) {
-                logger.info(e.getMessage(), e);
-            }
+        ResourceBundle bundle = getExternalResourceBundle(customBundleName, locale);
+        if (Objects.nonNull(bundle)) {
+            return bundle;
         }
         return ResourceBundle.getBundle(defaultBundleName, locale);
     }
 
-    ResourceBundle getBaseResources(String bundleName) {
+    /**
+     * Loads default resource bundle (from inside jar files).
+     * @param bundleName the bundle name
+     * @return the resource bundle
+     */
+    protected ResourceBundle getBaseResources(String bundleName) {
         return ResourceBundle.getBundle(bundleName, LocaleHelper.getCurrentLocale());
     }
 
-    Object getValueFromExtensionBundles(String key, String bundleName) {
-        ResourceBundle extensionResources = getExtensionResources(bundleName);
-        if (Objects.nonNull(extensionResources)) {
-            return extensionResources.getObject(key);
-        }
-        return null;
-    }
-
-    private ResourceBundle getExtensionResources(String bundleName) {
-        File file = new File(ConfigCore.getParameterOrDefaultValue(ParameterCore.DIR_LOCAL_MESSAGES));
-        if (file.exists()) {
-            try {
-                final URL resourceURL = file.toURI().toURL();
-                URLClassLoader urlLoader = AccessController.doPrivileged(
-                    (PrivilegedAction<URLClassLoader>) () -> new URLClassLoader(new URL[] {resourceURL }));
-                return ResourceBundle.getBundle(bundleName, LocaleHelper.getCurrentLocale(), urlLoader);
-            } catch (MalformedURLException | MissingResourceException e) {
-                logger.info(e.getMessage(), e);
-            }
+    /**
+     * Loads value from external resource bundles (outside jar files).
+     * @param key the key of the resource
+     * @param bundleName the bundle name
+     * @return the value or null if not exists
+     */
+    protected Object getValueFromExternalResourceBundle(String key, String bundleName) {
+        ResourceBundle bundle = getExternalResourceBundle(bundleName);
+        if (Objects.nonNull(bundle)) {
+            return bundle.getObject(key);
         }
         return null;
     }

Kitodo/src/main/java/org/kitodo/production/helper/messages/Error.java

@@ -23,10 +23,10 @@ public Enumeration<String> getKeys() {
 
     @Override
     protected Object handleGetObject(String key) {
-        // If there is an extension value use that
-        Object extensionValue = getValueFromExtensionBundles(key, "errors");
-        if (Objects.nonNull(extensionValue)) {
-            return extensionValue;
+        // If there is an external value use that
+        Object externalValue = getValueFromExternalResourceBundle(key, "errors");
+        if (Objects.nonNull(externalValue)) {
+            return externalValue;
         }
         // otherwise use the one defined in the property files
         return getBaseResources("messages.errors").getObject(key);

Kitodo/src/main/java/org/kitodo/production/helper/messages/Message.java

@@ -23,10 +23,10 @@ public Enumeration<String> getKeys() {
 
     @Override
     protected Object handleGetObject(String key) {
-        // If there is an extension value use that
-        Object extensionValue = getValueFromExtensionBundles(key, "messages");
-        if (Objects.nonNull(extensionValue)) {
-            return extensionValue;
+        // If there is an external value use that
+        Object externalValue = getValueFromExternalResourceBundle(key, "messages");
+        if (Objects.nonNull(externalValue)) {
+            return externalValue;
         }
         // otherwise use the one defined in the property files
         return getBaseResources("messages.messages").getObject(key);

Kitodo/src/test/java/org/kitodo/production/helper/messages/ErrorTest.java

@@ -11,29 +11,26 @@
 
 package org.kitodo.production.helper.messages;
 
-import java.io.File;
 import java.util.Enumeration;
 import java.util.Locale;
+import java.util.MissingResourceException;
 
 import org.junit.Test;
-import org.kitodo.FileLoader;
-import org.kitodo.production.services.ServiceManager;
-import org.kitodo.production.services.file.FileService;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
 
 public class ErrorTest {
 
-    private static final FileService fileService = ServiceManager.getFileService();
     private final Locale locale = new Locale("EN");
-    private final String customBundle = "errors";
+    private final String customBundle = "test_errors";
     private final String defaultBundle = "messages.errors";
 
     @Test
     public void shouldGetKeys() {
-        Enumeration<String> keys = Message.getResourceBundle(defaultBundle, customBundle, locale).getKeys();
+        Enumeration<String> keys = Error.getResourceBundle(defaultBundle, customBundle, locale).getKeys();
 
         boolean containsKey = false;
         while (keys.hasMoreElements()) {
@@ -49,23 +46,40 @@ public void shouldGetKeys() {
 
     @Test
     public void shouldGetStringFromDefaultBundle() {
-        assertEquals("Error...", Message.getResourceBundle(defaultBundle, customBundle, locale).getString("error"));
+        // in case custom bundle does not exist
+        assertEquals(
+            "Error...", 
+            Error.getResourceBundle(defaultBundle, "non-existent-bundle", locale).getString("error")
+        );
     }
 
     @Test
-    public void shouldGetStringFromCustomBundle() throws Exception {
-        File messageDirectory = new File("src/test/resources/custom");
+    public void shouldGetStringFromCustomBundle() {      
+        // in case custom bundle exists, and also contains definition for the requested key
+        assertEquals(
+            "Test custom error", 
+            Error.getResourceBundle(defaultBundle, customBundle, locale).getString("error")
+        );
+    }
 
-        if (messageDirectory.mkdir()) {
-            FileLoader.createCustomErrors();
+    @Test
+    public void shouldThrowMissingRessourceExceptionForNonExistentKey() {
+        // in case custom bundle is loaded, but key does not exist in either resource bundles 
+        assertThrows(
+            MissingResourceException.class, 
+            () -> Error.getResourceBundle(defaultBundle, customBundle, locale).getString("non-existent-key")
+        );
 
-            String value = Message.getResourceBundle(defaultBundle, customBundle, locale).getString("error");
-            assertEquals("Test custom error", value);
+        // in case custom bundle is missing, and key does also not exist in default bundle
+        assertThrows(
+            MissingResourceException.class, 
+            () -> Error.getResourceBundle(defaultBundle, "non-existent-bundle", locale).getString("non-existent-key")
+        );
 
-            FileLoader.deleteCustomErrors();
-            messageDirectory.delete();
-        } else {
-            fail("Directory for custom messages was not created!");
-        }
+        // in case custom bundle is loaded, but does not include the key, even if the key exists in the default bundle
+        assertThrows(
+            MissingResourceException.class, 
+            () -> Error.getResourceBundle(defaultBundle, customBundle, locale).getString("errorOccurred")
+        );
     }
 }

Kitodo/src/test/java/org/kitodo/production/helper/messages/MessageTest.java

@@ -11,24 +11,23 @@
 
 package org.kitodo.production.helper.messages;
 
-import java.io.File;
 import java.util.Enumeration;
 import java.util.Locale;
+import java.util.MissingResourceException;
 
 import org.junit.Test;
-import org.kitodo.FileLoader;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
 
 public class MessageTest {
 
     private final Locale locale = new Locale("EN");
-    private final String customBundle = "messages";
+    private final String customBundle = "test_messages";
     private final String defaultBundle = "messages.messages";
 
-
     @Test
     public void shouldGetKeys() {
         Enumeration<String> keys = Message.getResourceBundle(defaultBundle, customBundle, locale).getKeys();
@@ -47,23 +46,41 @@ public void shouldGetKeys() {
 
     @Test
     public void shouldGetStringFromDefaultBundle() {
-        assertEquals("Ready", Message.getResourceBundle(defaultBundle, customBundle, locale).getString("ready"));
+        // in case custom bundle does not exist
+        assertEquals(
+            "Ready", 
+            Message.getResourceBundle(defaultBundle, "non-existent-bundle", locale).getString("ready")
+        );
     }
 
     @Test
     public void shouldGetStringFromCustomBundle() throws Exception {
-        File messageDirectory = new File("src/test/resources/custom");
+        // in case custom bundle exists, and also contains definition for the requested key
+        assertEquals(
+            "Test custom message", 
+            Message.getResourceBundle(defaultBundle, customBundle, locale).getString("ready")
+        );
+    }
 
-        if (messageDirectory.mkdir()) {
-            FileLoader.createCustomMessages();
+    @Test
+    public void shouldThrowMissingRessourceExceptionForNonExistentKey() {
+        // in case custom bundle is loaded, but key does not exist in either resource bundles 
+        assertThrows(
+            MissingResourceException.class, 
+            () -> Message.getResourceBundle(defaultBundle, customBundle, locale).getString("non-existent-key")
+        );
 
-            String value = Message.getResourceBundle(defaultBundle, customBundle, locale).getString("ready");
-            assertEquals("Test custom message", value);
+        // in case custom bundle is missing, and key does also not exist in default bundle
+        assertThrows(
+            MissingResourceException.class, 
+            () -> Message.getResourceBundle(defaultBundle, "non-existent-bundle", locale).getString("non-existent-key")
+        );
 
-            FileLoader.deleteCustomMessages();
-            messageDirectory.delete();
-        } else {
-            fail("Directory for custom messages was not created!");
-        }
+        // in case custom bundle is loaded, but does not include the key, even if the key exists in the default bundle
+        assertThrows(
+            MissingResourceException.class, 
+            () -> Message.getResourceBundle(defaultBundle, customBundle, locale).getString("login")
+        );
     }
+
 }

Kitodo/src/test/resources/kitodo_config.properties

@@ -11,7 +11,7 @@
 # Absolute path to the directory that process directories will be created in,
 # terminated by a directory separator ("/").
 # The servlet container must have write permission to that directory.
-directory.messages=src/test/resources/custom/
+directory.messages=src/test/resources/messages/
 directory.metadata=src/test/resources/metadata/
 directory.rulesets=src/test/resources/rulesets/
 # Absolute path to the directory that XSLT files are stored in which are used

Kitodo/src/test/resources/messages/test_errors_en.properties

@@ -0,0 +1,2 @@
+custom=Test custom error
+error=Test custom error

Kitodo/src/test/resources/messages/test_messages_en.properties

@@ -0,0 +1,2 @@
+custom=Test custom message
+ready=Test custom message