fix: use Thread context ClassLoader for loading I18n ResourceBundle (#…

…19791) (#19821) ResourceBundle caches resources internally, this affects translation resources for the I18n feature. However, after a hot reload of a SpringBoot application in dev mode, up-to-date translations are expected in the output. With this change, a RestartClassLoader is used in SpringBoot applications for loading translation ResourceBundle, so that the translation bundles are reloaded on hot reload. Fixes vaadin/hilla#2554 Co-authored-by: Anton Platonov <[email protected]>
vaadin · Aug 23, 2024 · 58ab937 · 58ab937
1 parent e748c20
commit 58ab937
Show file tree

Hide file tree

Showing 6 changed files with 208 additions and 11 deletions.
diff --git a/flow-server/src/main/java/com/vaadin/flow/di/DefaultInstantiator.java b/flow-server/src/main/java/com/vaadin/flow/di/DefaultInstantiator.java
@@ -130,8 +130,7 @@ private I18NProvider getI18NProviderInstance() {
         try {
             // Get i18n provider class if found in application
             // properties
-            Class<?> providerClass = DefaultInstantiator.class.getClassLoader()
-                    .loadClass(property);
+            Class<?> providerClass = getClassLoader().loadClass(property);
             if (I18NProvider.class.isAssignableFrom(providerClass)) {
 
                 return ReflectTools.createInstance(
@@ -154,8 +153,7 @@ private MenuAccessControl getMenuAccessControlInstance() {
         try {
             // Get Menu Access Control class if found in application
             // properties
-            Class<?> providerClass = DefaultInstantiator.class.getClassLoader()
-                    .loadClass(property);
+            Class<?> providerClass = getClassLoader().loadClass(property);
             if (MenuAccessControl.class.isAssignableFrom(providerClass)) {
 
                 return ReflectTools.createInstance(
@@ -175,7 +173,10 @@ private MenuAccessControl getMenuAccessControlInstance() {
     }
 
     protected ClassLoader getClassLoader() {
-        return getClass().getClassLoader();
+        // Use the application thread ClassLoader to invalidate ResourceBundle
+        // cache on dev mode reload. See
+        // https://github.com/vaadin/hilla/issues/2554
+        return Thread.currentThread().getContextClassLoader();
     }
 
     /**

diff --git a/flow-server/src/test/java/com/vaadin/flow/i18n/DefaultInstantiatorI18NTest.java b/flow-server/src/test/java/com/vaadin/flow/i18n/DefaultInstantiatorI18NTest.java
@@ -33,6 +33,8 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
+import org.mockito.Mock;
+import org.mockito.MockedConstruction;
 import org.mockito.Mockito;
 
 import com.vaadin.flow.di.DefaultInstantiator;
@@ -176,6 +178,45 @@ protected ClassLoader getClassLoader() {
                 i18NProvider.getTranslation("title", new Locale("en", "GB")));
     }
 
+    @Test
+    public void translationFilesOnClassPath_getI18NProvider_usesThreadContextClassLoader()
+            throws IOException {
+        createTranslationFiles(translations);
+
+        VaadinService service = Mockito.mock(VaadinService.class);
+        mockLookup(service);
+        VaadinService.setCurrent(service);
+
+        DefaultInstantiator defaultInstantiator = new DefaultInstantiator(
+                service);
+        Mockito.when(service.getInstantiator()).thenReturn(defaultInstantiator);
+
+        ClassLoader threadContextClassLoader = Thread.currentThread()
+                .getContextClassLoader();
+        try {
+            Thread.currentThread().setContextClassLoader(urlClassLoader);
+
+            try (MockedConstruction<DefaultI18NProvider> mockedConstruction = Mockito
+                    .mockConstruction(DefaultI18NProvider.class,
+                            (mock, context) -> {
+                                ClassLoader classLoaderArgument = (ClassLoader) context
+                                        .arguments().get(1);
+                                Assert.assertEquals(urlClassLoader,
+                                        classLoaderArgument);
+                            })) {
+                I18NProvider i18NProvider = defaultInstantiator
+                        .getI18NProvider();
+
+                Assert.assertNotNull(i18NProvider);
+                Assert.assertEquals(i18NProvider,
+                        mockedConstruction.constructed().get(0));
+            }
+        } finally {
+            Thread.currentThread()
+                    .setContextClassLoader(threadContextClassLoader);
+        }
+    }
+
     private static void createTranslationFiles(File translationsFolder)
             throws IOException {
         File file = new File(translationsFolder,

diff --git a/...r/src/test/java/com/vaadin/flow/server/auth/DefaultInstantiatorMenuAccessControlTest.java b/...r/src/test/java/com/vaadin/flow/server/auth/DefaultInstantiatorMenuAccessControlTest.java
@@ -19,9 +19,12 @@
 import java.lang.reflect.Field;
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.AdditionalAnswers;
+import org.mockito.Answers;
 import org.mockito.Mockito;
 
 import com.vaadin.flow.di.DefaultInstantiator;
@@ -30,18 +33,36 @@
 import com.vaadin.flow.server.InvalidMenuAccessControlException;
 import com.vaadin.flow.server.VaadinContext;
 import com.vaadin.flow.server.VaadinService;
+import org.mockito.invocation.InvocationOnMock;
 
 import static org.junit.Assert.assertThrows;
+import net.jcip.annotations.NotThreadSafe;
 
+@NotThreadSafe
 public class DefaultInstantiatorMenuAccessControlTest {
+    private ClassLoader contextClassLoader;
+    private ClassLoader classLoader;
 
     @Before
-    public void init() throws NoSuchFieldException, IllegalAccessException {
+    public void init() throws NoSuchFieldException, IllegalAccessException,
+            ClassNotFoundException {
         clearMenuAccessControlField();
+        contextClassLoader = Thread.currentThread().getContextClassLoader();
+
+        classLoader = Mockito.mock(ClassLoader.class);
+        Mockito.when(classLoader.loadClass(Mockito.any()))
+                .thenAnswer(AdditionalAnswers.delegatesTo(contextClassLoader));
+        Thread.currentThread().setContextClassLoader(classLoader);
+    }
+
+    @After
+    public void destroy() throws NoSuchFieldException, IllegalAccessException {
+        Thread.currentThread().setContextClassLoader(contextClassLoader);
     }
 
     @Test
-    public void defaultInstantiator_getMenuAccessControl_defaultMenuAccessControl() {
+    public void defaultInstantiator_getMenuAccessControl_defaultMenuAccessControl()
+            throws ClassNotFoundException {
         VaadinService service = Mockito.mock(VaadinService.class);
         mockLookup(service);
         DefaultInstantiator defaultInstantiator = new DefaultInstantiator(
@@ -56,14 +77,17 @@ public void defaultInstantiator_getMenuAccessControl_defaultMenuAccessControl()
     }
 
     @Test
-    public void defaultInstantiator_getMenuAccessControl_customMenuAccessControl() {
+    public void defaultInstantiator_getMenuAccessControl_customMenuAccessControl()
+            throws ClassNotFoundException {
+        String customMenuAccessControlClassName = "com.vaadin.flow.server.auth.CustomMenuAccessControl";
+
         VaadinService service = Mockito.mock(VaadinService.class);
         mockLookup(service);
         DefaultInstantiator defaultInstantiator = new DefaultInstantiator(
                 service) {
             @Override
             protected String getInitProperty(String propertyName) {
-                return "com.vaadin.flow.server.auth.CustomMenuAccessControl";
+                return customMenuAccessControlClassName;
             }
         };
         MenuAccessControl menuAccessControl = defaultInstantiator
@@ -72,6 +96,8 @@ protected String getInitProperty(String propertyName) {
         Assert.assertTrue(menuAccessControl instanceof CustomMenuAccessControl);
         Assert.assertSame(menuAccessControl.getPopulateClientSideMenu(),
                 MenuAccessControl.PopulateClientMenu.ALWAYS);
+
+        Mockito.verify(classLoader).loadClass(customMenuAccessControlClassName);
     }
 
     @Test

diff --git a/vaadin-spring/src/main/java/com/vaadin/flow/spring/i18n/DefaultI18NProviderFactory.java b/vaadin-spring/src/main/java/com/vaadin/flow/spring/i18n/DefaultI18NProviderFactory.java
@@ -66,8 +66,12 @@ public static DefaultI18NProvider create(String locationPattern) {
                         Arrays.stream(translations).map(Resource::getFilename)
                                 .filter(Objects::nonNull)
                                 .collect(Collectors.toList()));
-                return new DefaultI18NProvider(locales,
-                        DefaultI18NProviderFactory.class.getClassLoader());
+                // Makes use of the RestartClassLoader to invalidate the
+                // ResourceBundle cache on SpringBoot application dev mode
+                // reload. See https://github.com/vaadin/hilla/issues/2554
+                ClassLoader classLoader = Thread.currentThread()
+                        .getContextClassLoader();
+                return new DefaultI18NProvider(locales, classLoader);
             }
         } catch (IOException e) {
             LoggerFactory.getLogger(DefaultI18NProviderFactory.class)

diff --git a/vaadin-spring/src/test/java/com/vaadin/flow/spring/i18n/DefaultI18NProviderFactorySuite.java b/vaadin-spring/src/test/java/com/vaadin/flow/spring/i18n/DefaultI18NProviderFactorySuite.java
@@ -0,0 +1,11 @@
+package com.vaadin.flow.spring.i18n;
+
+import net.jcip.annotations.NotThreadSafe;
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+@RunWith(Suite.class)
+@NotThreadSafe
+@Suite.SuiteClasses({ DefaultI18NProviderFactoryTest.class })
+public class DefaultI18NProviderFactorySuite {
+}
diff --git a/vaadin-spring/src/test/java/com/vaadin/flow/spring/i18n/DefaultI18NProviderFactoryTest.java b/vaadin-spring/src/test/java/com/vaadin/flow/spring/i18n/DefaultI18NProviderFactoryTest.java
@@ -0,0 +1,114 @@
+package com.vaadin.flow.spring.i18n;
+
+import com.vaadin.flow.di.Instantiator;
+import com.vaadin.flow.i18n.DefaultI18NProvider;
+import com.vaadin.flow.i18n.I18NProvider;
+import com.vaadin.flow.spring.VaadinApplicationConfiguration;
+import com.vaadin.flow.spring.instantiator.SpringInstantiatorTest;
+import jakarta.servlet.ServletException;
+import net.jcip.annotations.NotThreadSafe;
+import org.junit.*;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.mockito.MockedConstruction;
+import org.mockito.Mockito;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.annotation.*;
+import org.springframework.core.io.DefaultResourceLoader;
+import org.springframework.core.io.Resource;
+import org.springframework.core.io.support.PathMatchingResourcePatternResolver;
+import org.springframework.test.context.junit4.SpringRunner;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.StandardOpenOption;
+import java.util.Locale;
+import java.util.Properties;
+
+@RunWith(SpringRunner.class)
+@Import(VaadinApplicationConfiguration.class)
+@NotThreadSafe
+public class DefaultI18NProviderFactoryTest {
+
+    @Autowired
+    private ApplicationContext context;
+
+    static private ClassLoader originalClassLoader;
+
+    static private ClassLoader testClassLoader;
+
+    static private TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+    static volatile private MockedConstruction<PathMatchingResourcePatternResolver> pathMatchingResourcePatternResolverMockedConstruction;
+
+    @BeforeClass
+    static public void setup() throws IOException {
+        originalClassLoader = Thread.currentThread().getContextClassLoader();
+
+        temporaryFolder.create();
+        File resources = temporaryFolder.newFolder();
+
+        File translations = new File(resources,
+                DefaultI18NProvider.BUNDLE_FOLDER);
+        translations.mkdirs();
+
+        File defaultTranslation = new File(translations,
+                DefaultI18NProvider.BUNDLE_FILENAME + ".properties");
+        Files.writeString(defaultTranslation.toPath(), "title=Default lang",
+                StandardCharsets.UTF_8, StandardOpenOption.CREATE);
+
+        testClassLoader = new URLClassLoader(
+                new URL[] { resources.toURI().toURL() },
+                DefaultI18NProviderFactory.class.getClassLoader());
+        Thread.currentThread().setContextClassLoader(testClassLoader);
+
+        Resource translationResource = new DefaultResourceLoader()
+                .getResource(DefaultI18NProvider.BUNDLE_FOLDER + "/"
+                        + DefaultI18NProvider.BUNDLE_FILENAME + ".properties");
+
+        pathMatchingResourcePatternResolverMockedConstruction = Mockito
+                .mockConstruction(PathMatchingResourcePatternResolver.class,
+                        (mock, context) -> {
+                            Mockito.when(mock.getPathMatcher())
+                                    .thenCallRealMethod();
+                            Mockito.when(mock.getResources(Mockito.anyString()))
+                                    .thenAnswer(invocationOnMock -> {
+                                        String pattern = invocationOnMock
+                                                .getArgument(0);
+                                        Assert.assertEquals(
+                                                "classpath*:/vaadin-i18n/*.properties",
+                                                pattern);
+                                        return new Resource[] {
+                                                translationResource };
+                                    });
+                        });
+    }
+
+    @AfterClass
+    static public void teardown() throws Exception {
+        pathMatchingResourcePatternResolverMockedConstruction.close();
+        Thread.currentThread().setContextClassLoader(originalClassLoader);
+    }
+
+    @Test
+    public void create_usesThreadContextClassLoader() throws ServletException {
+        Instantiator instantiator = getInstantiator(context);
+        I18NProvider i18NProvider = instantiator.getI18NProvider();
+
+        Assert.assertNotNull(i18NProvider);
+        Assert.assertTrue(i18NProvider instanceof DefaultI18NProvider);
+        Assert.assertEquals("Default lang",
+                i18NProvider.getTranslation("title", Locale.getDefault()));
+    }
+
+    private static Instantiator getInstantiator(ApplicationContext context)
+            throws ServletException {
+        return SpringInstantiatorTest.getService(context, new Properties())
+                .getInstantiator();
+    }
+}