Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use Thread context ClassLoader for loading I18n ResourceBundle #19791

Merged
merged 11 commits into from
Aug 23, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -217,6 +219,45 @@ public void translate_withoutInstantiator_throwsIllegalStateException() {
() -> I18NProvider.translate("foo.bar"));
}

@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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should perhaps be marked with @NotThreadSafe as we use a set current in the test

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(
Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Loading