[Improve] Optimize ResourceBundle & add unit test. (#2297)

Signed-off-by: yuluo-yx <[email protected]> Co-authored-by: tomsun28 <[email protected]>
apache · Jul 19, 2024 · 55b3dd7 · 55b3dd7
1 parent 91111e5
commit 55b3dd7
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 43 deletions.
diff --git a/common/src/main/java/org/apache/hertzbeat/common/support/ResourceBundleUtf8Control.java b/common/src/main/java/org/apache/hertzbeat/common/support/ResourceBundleUtf8Control.java
@@ -20,21 +20,18 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.lang.reflect.InvocationTargetException;
 import java.net.URL;
 import java.net.URLConnection;
 import java.nio.charset.StandardCharsets;
-import java.security.AccessController;
-import java.security.PrivilegedActionException;
-import java.security.PrivilegedExceptionAction;
 import java.util.Locale;
 import java.util.PropertyResourceBundle;
 import java.util.ResourceBundle;
-import lombok.extern.slf4j.Slf4j;
 
 /**
  * i18n resource bundle control
  */
-@Slf4j
+
 public class ResourceBundleUtf8Control extends ResourceBundle.Control {
 
     private static final String JAVA_CLASS = "java.class";
@@ -44,6 +41,7 @@ public class ResourceBundleUtf8Control extends ResourceBundle.Control {
     @Override
     public ResourceBundle newBundle(String baseName, Locale locale, String format, ClassLoader loader, boolean reload)
             throws IllegalAccessException, InstantiationException, IOException {
+
         String bundleName = toBundleName(baseName, locale);
         ResourceBundle bundle = null;
         if (JAVA_CLASS.equals(format)) {
@@ -55,48 +53,25 @@ public ResourceBundle newBundle(String baseName, Locale locale, String format, C
                 // If the class isn't a ResourceBundle subclass, throw a
                 // ClassCastException.
                 if (ResourceBundle.class.isAssignableFrom(bundleClass)) {
-                    bundle = bundleClass.newInstance();
+                    bundle = bundleClass.getDeclaredConstructor().newInstance();
                 } else {
                     throw new ClassCastException(bundleClass.getName()
                             + " cannot be cast to ResourceBundle");
                 }
             } catch (ClassNotFoundException ignored) {}
-        } else if (JAVA_PROPERTIES.equals(format)) {
-            final String resourceName = toResourceName0(bundleName, "properties");
+            catch (InvocationTargetException | NoSuchMethodException e) {
+                throw new RuntimeException(e);
+            }
+	} else if (JAVA_PROPERTIES.equals(format)) {
+            final String resourceName = toResourceName0(bundleName);
             if (resourceName == null) {
                 return null;
             }
-            final ClassLoader classLoader = loader;
-            final boolean reloadFlag = reload;
-            InputStream stream;
-            try {
-                stream = AccessController.doPrivileged(
-                        (PrivilegedExceptionAction<InputStream>) () -> {
-                            InputStream is = null;
-                            if (reloadFlag) {
-                                URL url = classLoader.getResource(resourceName);
-                                if (url != null) {
-                                    URLConnection connection = url.openConnection();
-                                    if (connection != null) {
-                                        // Disable caches to get fresh data for
-                                        // reloading.
-                                        connection.setUseCaches(false);
-                                        is = connection.getInputStream();
-                                    }
-                                }
-                            } else {
-                                is = classLoader.getResourceAsStream(resourceName);
-                            }
-                            return is;
-                        });
-            } catch (PrivilegedActionException e) {
-                throw (IOException) e.getException();
-            }
+	    InputStream stream = getResourceInputStream(loader, resourceName, reload);
+
             if (stream != null) {
-                try {
+                try (stream) {
                     bundle = new PropertyResourceBundle(new InputStreamReader(stream, StandardCharsets.UTF_8));
-                } finally {
-                    stream.close();
                 }
             }
         } else {
@@ -105,12 +80,34 @@ public ResourceBundle newBundle(String baseName, Locale locale, String format, C
         return bundle;
     }
 
-    private String toResourceName0(String bundleName, String suffix) {
+    private String toResourceName0(String bundleName) {
         // application protocol check
         if (bundleName.contains(SPILT)) {
             return null;
         } else {
-            return toResourceName(bundleName, suffix);
+            return toResourceName(bundleName, "properties");
+        }
+    }
+
+    private InputStream getResourceInputStream(ClassLoader classLoader, String resourceName, boolean reloadFlag) throws IOException {
+
+        InputStream is = null;
+
+        if (reloadFlag) {
+            URL url = classLoader.getResource(resourceName);
+            if (url != null) {
+                URLConnection connection = url.openConnection();
+                if (connection != null) {
+                    // Disable caches to get fresh data for reloading.
+                    connection.setUseCaches(false);
+                    is = connection.getInputStream();
+                }
+            }
+        } else {
+            is = classLoader.getResourceAsStream(resourceName);
         }
+
+        return is;
     }
+
 }
diff --git a/common/src/test/java/org/apache/hertzbeat/common/support/ResourceBundleUtf8ControlTest.java b/common/src/test/java/org/apache/hertzbeat/common/support/ResourceBundleUtf8ControlTest.java
@@ -17,14 +17,59 @@
 
 package org.apache.hertzbeat.common.support;
 
+import java.io.IOException;
+import java.util.Locale;
+import java.util.ResourceBundle;
+
 import org.junit.jupiter.api.Test;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
 /**
  * Test case for {@link ResourceBundleUtf8Control}
  */
 class ResourceBundleUtf8ControlTest {
 
-    @Test
-    void newBundle() {
-    }
-}
+	@Test
+	void testNewBundleWithPropertiesFormat() throws IllegalAccessException, InstantiationException, IOException {
+
+        ResourceBundle.Control control = new ResourceBundleUtf8Control();
+		ClassLoader loader = getClass().getClassLoader();
+		String baseName = "msg";
+
+		ResourceBundle bundle = control.newBundle(baseName, Locale.ENGLISH, "java.properties", loader, false);
+        assertNotNull(bundle);
+		assertEquals("Hello, World!", bundle.getString("hello"));
+
+		bundle = control.newBundle(baseName, Locale.ROOT, "java.properties", loader, false);
+		assertNotNull(bundle);
+		assertEquals("你好", bundle.getString("hello"));
+	}
+
+	@Test
+	void testNewBundleWithClassFormat() throws IllegalAccessException, InstantiationException, IOException {
+
+		ResourceBundle.Control control = new ResourceBundleUtf8Control();
+		ClassLoader loader = getClass().getClassLoader();
+		String baseName = "dummyClassBundle";
+
+		ResourceBundle bundle = control.newBundle(baseName, Locale.ENGLISH, "java.class", loader, false);
+		//because not have an actual class, bundle should be null
+		assertNull(bundle);
+	}
+
+	@Test
+	void testReloading() throws IllegalAccessException, InstantiationException, IOException {
+		ResourceBundle.Control control = new ResourceBundleUtf8Control();
+		ClassLoader loader = getClass().getClassLoader();
+		String baseName = "msg";
+
+		// Test with reload flag
+		ResourceBundle bundle = control.newBundle(baseName, Locale.ENGLISH, "java.properties", loader, true);
+		assertNotNull(bundle);
+		assertEquals("Hello, World!", bundle.getString("hello"));
+	}
+
+}
diff --git a/common/src/test/resources/msg.properties b/common/src/test/resources/msg.properties
@@ -0,0 +1,16 @@
+# 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.
+
+hello=你好
diff --git a/common/src/test/resources/msg_en.properties b/common/src/test/resources/msg_en.properties
@@ -0,0 +1,16 @@
+# 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.
+
+hello=Hello, World!