This is an automated email from the ASF dual-hosted git repository. adelbene pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 755a63ac249f3864c3a7692cce4712b6b358a7ee Author: Pedro Santos <[email protected]> AuthorDate: Fri Nov 1 00:35:36 2024 -0300 WICKET-7024 squash to the last commit (WIP) - test cases - code refactoring - WIP: the fix works only to mounted resources, next step is to make it work with the BasicResourceReferenceMapper --- .../resource/PackageResourceReferenceTest.java | 52 +++++++++++++- .../locator/CachingResourceStreamLocatorTest.java | 14 ++-- .../mapper/BasicResourceReferenceMapper.java | 7 +- .../resource/locator/IResourceStreamLocator.java | 24 ------- .../resource/locator/ResourceStreamLocator.java | 15 ---- .../caching/CachingResourceStreamLocator.java | 3 +- .../wicket/request/resource/PackageResource.java | 80 +++++++++++++++++++++- .../request/resource/PackageResourceReference.java | 35 +--------- 8 files changed, 142 insertions(+), 88 deletions(-) diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/core/request/resource/PackageResourceReferenceTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/core/request/resource/PackageResourceReferenceTest.java index 657423f877..922a192e22 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/core/request/resource/PackageResourceReferenceTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/core/request/resource/PackageResourceReferenceTest.java @@ -24,6 +24,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.*; +import static org.mockito.Mockito.times; import java.io.IOException; import java.io.InputStream; @@ -32,6 +34,9 @@ import java.util.Locale; import org.apache.wicket.Application; import org.apache.wicket.MarkupContainer; import org.apache.wicket.ThreadContext; +import org.apache.wicket.core.util.resource.UrlResourceStream; +import org.apache.wicket.core.util.resource.locator.IResourceStreamLocator; +import org.apache.wicket.core.util.resource.locator.caching.CachingResourceStreamLocator; import org.apache.wicket.markup.IMarkupResourceStreamProvider; import org.apache.wicket.markup.html.WebPage; import org.apache.wicket.protocol.http.mock.MockHttpServletRequest; @@ -50,10 +55,12 @@ import org.apache.wicket.request.resource.ResourceReference; import org.apache.wicket.request.resource.ResourceReference.UrlAttributes; import org.apache.wicket.response.ByteArrayResponse; import org.apache.wicket.util.io.IOUtils; +import org.apache.wicket.util.resource.FileResourceStream; import org.apache.wicket.util.resource.IResourceStream; import org.apache.wicket.util.resource.StringResourceStream; import org.apache.wicket.util.tester.WicketTestCase; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; /** @@ -62,6 +69,7 @@ import org.junit.jupiter.api.Test; class PackageResourceReferenceTest extends WicketTestCase { private static Class<PackageResourceReferenceTest> scope = PackageResourceReferenceTest.class; + private static final Locale defaultLocale = Locale.CHINA; private static final Locale[] locales = { null, new Locale("en"), new Locale("en", "US") }; private static final String[] styles = { null, "style" }; private static final String[] variations = { null, "var" }; @@ -406,7 +414,7 @@ class PackageResourceReferenceTest extends WicketTestCase } @Test - public void getResouceWithNoStyle() + public void getResourceWithNoStyle() { tester.executeUrl( "wicket/resource/org.apache.wicket.core.request.resource.PackageResourceReferenceTest/a.css"); @@ -435,6 +443,48 @@ class PackageResourceReferenceTest extends WicketTestCase assertThat(tester.getLastResponseAsString(), not(containsString("blue"))); } + @Test + @Disabled + public void doNotFindResourceInTheCache() + { + IResourceStreamLocator resourceStreamLocator = mock(IResourceStreamLocator.class); + when(resourceStreamLocator.locate(scope, "org/apache/wicket/core/request/resource/a.css", + "yellow", null, defaultLocale, null, false)).thenReturn( + new UrlResourceStream(scope.getResource("a.css"))); + + tester.getApplication().getResourceSettings() + .setResourceStreamLocator(new CachingResourceStreamLocator(resourceStreamLocator)); + + tester.executeUrl( + "wicket/resource/org.apache.wicket.core.request.resource.PackageResourceReferenceTest/a.css?-yellow"); + tester.executeUrl( + "wicket/resource/org.apache.wicket.core.request.resource.PackageResourceReferenceTest/a.css?-yellow"); + + verify(resourceStreamLocator, times(2)).locate(PackageResourceReferenceTest.class, + "org/apache/wicket/core/request/resource/a.css", "yellow", null, defaultLocale, null, false); + } + + @Test + public void doNotFindMountedResourceInTheCache() + { + IResourceStreamLocator resourceStreamLocator = mock(IResourceStreamLocator.class); + when(resourceStreamLocator.locate(scope, "org/apache/wicket/core/request/resource/a.css", + "yellow", null, defaultLocale, null, false)).thenReturn( + new UrlResourceStream(scope.getResource("a.css"))); + + tester.getApplication().getResourceSettings() + .setResourceStreamLocator(new CachingResourceStreamLocator(resourceStreamLocator)); + tester.getApplication() + .mountResource("/a.css", new PackageResourceReference(scope, "a.css")); + + tester.executeUrl("a.css?-yellow"); + tester.executeUrl("a.css?-yellow"); + + verify(resourceStreamLocator, times(2)).locate(scope, + "org/apache/wicket/core/request/resource/a.css", "yellow", null, defaultLocale, null, + false); + } + /** * @see https://issues.apache.org/jira/browse/WICKET-7024 */ diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/core/util/resource/locator/CachingResourceStreamLocatorTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/core/util/resource/locator/CachingResourceStreamLocatorTest.java index 6b1f55c71f..fd15e29cb3 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/core/util/resource/locator/CachingResourceStreamLocatorTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/core/util/resource/locator/CachingResourceStreamLocatorTest.java @@ -168,7 +168,7 @@ class CachingResourceStreamLocatorTest FileResourceStream frs = new FileResourceStream(new File(".")); when(resourceStreamLocator.locate(String.class, "path", "style", "variation", null, - "extension", true, true)).thenReturn(frs); + "extension", true)).thenReturn(frs); CachingResourceStreamLocator cachingLocator = new CachingResourceStreamLocator( resourceStreamLocator); @@ -178,7 +178,7 @@ class CachingResourceStreamLocatorTest // there is a file resource with that Key so expect just one call to the delegate verify(resourceStreamLocator, times(1)).locate(String.class, "path", "style", "variation", - null, "extension", true, true); + null, "extension", true); } /** @@ -192,7 +192,7 @@ class CachingResourceStreamLocatorTest FileResourceStream frs = new FileResourceStream(new File(".")); when(resourceStreamLocator.locate(String.class, "path", "style", "variation", null, - "extension", true, true)).thenReturn(frs); + "extension", true)).thenReturn(frs); CachingResourceStreamLocator cachingLocator = new CachingResourceStreamLocator( resourceStreamLocator); @@ -203,9 +203,9 @@ class CachingResourceStreamLocatorTest // there is a file resource with that Key so expect just one call to the delegate verify(resourceStreamLocator, times(1)).locate(String.class, "path", "style", "variation", - null, "extension", true, true); + null, "extension", true); verify(resourceStreamLocator, times(1)).locate(String.class, "path", "style", "variation", - null, "extension2", true, true); + null, "extension2", true); } /** @@ -244,7 +244,7 @@ class CachingResourceStreamLocatorTest StringResourceStream srs = new StringResourceStream("anything"); when(resourceStreamLocator.locate(String.class, "path", "style", "variation", null, - "extension", true, true)).thenReturn(srs); + "extension", true)).thenReturn(srs); CachingResourceStreamLocator cachingLocator = new CachingResourceStreamLocator( resourceStreamLocator); @@ -255,6 +255,6 @@ class CachingResourceStreamLocatorTest // lightweight resource streams should not be cached so expect just a call to the delegate // for each call to the caching locator verify(resourceStreamLocator, times(2)).locate(String.class, "path", "style", "variation", - null, "extension", true, true); + null, "extension", true); } } diff --git a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/BasicResourceReferenceMapper.java b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/BasicResourceReferenceMapper.java index 2c533bc59b..320796d710 100755 --- a/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/BasicResourceReferenceMapper.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/request/mapper/BasicResourceReferenceMapper.java @@ -27,10 +27,7 @@ import org.apache.wicket.request.Url; import org.apache.wicket.request.handler.resource.ResourceReferenceRequestHandler; import org.apache.wicket.request.mapper.parameter.IPageParametersEncoder; import org.apache.wicket.request.mapper.parameter.PageParameters; -import org.apache.wicket.request.resource.IResource; -import org.apache.wicket.request.resource.MetaInfStaticResourceReference; -import org.apache.wicket.request.resource.ResourceReference; -import org.apache.wicket.request.resource.ResourceReferenceRegistry; +import org.apache.wicket.request.resource.*; import org.apache.wicket.request.resource.caching.IResourceCachingStrategy; import org.apache.wicket.request.resource.caching.IStaticCacheableResource; import org.apache.wicket.request.resource.caching.ResourceUrl; @@ -133,6 +130,8 @@ public class BasicResourceReferenceMapper extends AbstractResourceReferenceMappe Class<?> scope = resolveClass(className); + // attributes = PackageResource.sanitize(attributes, scope, name.toString()); + if (scope != null && scope.getPackage() != null) { ResourceReference res = getContext().getResourceReferenceRegistry() diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/IResourceStreamLocator.java b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/IResourceStreamLocator.java index 0521845d24..c0a34e38b9 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/IResourceStreamLocator.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/IResourceStreamLocator.java @@ -64,34 +64,10 @@ public interface IResourceStreamLocator * @param strict * whether the specified attributes must match exactly * @return The resource or null - * @deprecated */ IResourceStream locate(Class<?> clazz, String path, String style, String variation, Locale locale, String extension, boolean strict); - /** - * Locate a resource by combining the given path, style, variation, locale and extension - * parameters. The exact search order depends on the implementation. - * - * @param clazz - * The class loader for delegating the loading of the resource - * @param path - * The path of the resource - * @param style - * Any resource style, such as a skin style (see {@link org.apache.wicket.Session}) - * @param variation - * The component's variation (of the style) - * @param locale - * The locale of the resource to load - * @param extension - * A comma separate list of extensions - * @param strict - * whether the specified attributes must match exactly - * @return The resource or null - */ - IResourceStream locate(Class<?> clazz, String path, String style, String variation, - Locale locale, String extension, boolean strict, boolean updateCache); - /** * Markup resources and Properties files both need to iterate over different combinations of * locale, style, etc.. And though no single locate(..) method exists which is used by both, diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ResourceStreamLocator.java b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ResourceStreamLocator.java index f59e6ec138..55a15c449c 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ResourceStreamLocator.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ResourceStreamLocator.java @@ -140,25 +140,10 @@ public class ResourceStreamLocator implements IResourceStreamLocator * @see org.apache.wicket.core.util.resource.locator.IResourceStreamLocator#locate(java.lang.Class, * java.lang.String, java.lang.String, java.lang.String, java.util.Locale, * java.lang.String, boolean) - * @deprecated */ @Override public IResourceStream locate(final Class<?> clazz, String path, final String style, final String variation, Locale locale, final String extension, final boolean strict) - { - return locate(clazz, path, style, variation, locale, extension, strict, true); - } - - /** - * - * @see org.apache.wicket.core.util.resource.locator.IResourceStreamLocator#locate(java.lang.Class, - * java.lang.String, java.lang.String, java.lang.String, java.util.Locale, - * java.lang.String, boolean) - */ - @Override - public IResourceStream locate(final Class<?> clazz, String path, final String style, - final String variation, Locale locale, final String extension, final boolean strict, - boolean updateCache) { // If path contains a locale, then it'll replace the locale provided to this method PathLocale data = ResourceUtils.getLocaleFromFilename(path); diff --git a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/caching/CachingResourceStreamLocator.java b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/caching/CachingResourceStreamLocator.java index 6eed118ab9..7cc3a45684 100644 --- a/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/caching/CachingResourceStreamLocator.java +++ b/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/caching/CachingResourceStreamLocator.java @@ -119,7 +119,6 @@ public class CachingResourceStreamLocator implements IResourceStreamLocator return locate(scope, path, style, variation, locale, extension, strict, true); } - @Override public IResourceStream locate(Class<?> scope, String path, String style, String variation, Locale locale, String extension, boolean strict, boolean updateCache) { @@ -129,7 +128,7 @@ public class CachingResourceStreamLocator implements IResourceStreamLocator final IResourceStream result; if (resourceStreamReference == null) { - result = delegate.locate(scope, path, style, variation, locale, extension, strict, updateCache); + result = delegate.locate(scope, path, style, variation, locale, extension, strict); if (updateCache) { diff --git a/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResource.java b/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResource.java index 63d9eda185..06c20f588c 100644 --- a/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResource.java +++ b/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResource.java @@ -32,6 +32,7 @@ import org.apache.wicket.Session; import org.apache.wicket.WicketRuntimeException; import org.apache.wicket.core.util.lang.WicketObjects; import org.apache.wicket.core.util.resource.locator.IResourceStreamLocator; +import org.apache.wicket.core.util.resource.locator.caching.CachingResourceStreamLocator; import org.apache.wicket.javascript.IJavaScriptCompressor; import org.apache.wicket.markup.html.IPackageResourceGuard; import org.apache.wicket.mock.MockWebRequest; @@ -149,6 +150,13 @@ public class PackageResource extends AbstractResource implements IStaticCacheabl */ private boolean cachingEnabled = true; + /** + * controls whether + * {@link org.apache.wicket.core.util.resource.locator.caching.CachingResourceStreamLocator} + * should update the cache + */ + private boolean serverResourceStreamReferenceCacheUpdate = true; + /** * text encoding (may be null) - only makes sense for character-based resources */ @@ -240,6 +248,27 @@ public class PackageResource extends AbstractResource implements IStaticCacheabl this.cachingEnabled = enabled; } + /** + * Returns true if the cache should be updated for this resource + * + * @return if the cache update is enabled + */ + public boolean isServerResourceStreamReferenceCacheUpdate() + { + return serverResourceStreamReferenceCacheUpdate; + } + + /** + * Sets the cache update for this resource to be enabled + * + * @param enabled + * if the cache update should be enabled + */ + public void setServerResourceStreamReferenceCacheUpdate(final boolean enabled) + { + this.serverResourceStreamReferenceCacheUpdate = enabled; + } + /** * get text encoding (intended for character-based resources) * @@ -531,7 +560,7 @@ public class PackageResource extends AbstractResource implements IStaticCacheabl @Override public IResourceStream getResourceStream() { - return internalGetResourceStream(getCurrentStyle(), getCurrentLocale(), isCachingEnabled()); + return internalGetResourceStream(getCurrentStyle(), getCurrentLocale(), isServerResourceStreamReferenceCacheUpdate()); } /** @@ -557,8 +586,18 @@ public class PackageResource extends AbstractResource implements IStaticCacheabl IResourceStreamLocator resourceStreamLocator = Application.get() .getResourceSettings() .getResourceStreamLocator(); - IResourceStream resourceStream = resourceStreamLocator.locate(getScope(), absolutePath, - style, variation, locale, null, false, updateCache); + IResourceStream resourceStream = null; + + if (resourceStreamLocator instanceof CachingResourceStreamLocator cache) + { + resourceStream = cache.locate(getScope(), absolutePath, style, variation, locale, null, + false, updateCache); + } + else + { + resourceStream = resourceStreamLocator.locate(getScope(), absolutePath, style, + variation, locale, null, false); + } String realPath = absolutePath; if (resourceStream instanceof IFixedLocationResourceStream) @@ -856,4 +895,39 @@ public class PackageResource extends AbstractResource implements IStaticCacheabl return this; } + public static ResourceReference.UrlAttributes sanitize( + ResourceReference.UrlAttributes urlAttributes, Class<?> scope, String name) + { + PackageResource urlResource = new PackageResource(scope, name, urlAttributes.getLocale(), + urlAttributes.getStyle(), urlAttributes.getVariation()); + urlResource.setServerResourceStreamReferenceCacheUpdate(false); + IResourceStream filesystemMatch = urlResource.getResourceStream(); + + ResourceReference.Key urlKey = new ResourceReference.Key(scope.getName(), name, + urlAttributes.getLocale(), urlAttributes.getStyle(), urlAttributes.getVariation()); + + ResourceReference.Key filesystemKey = new ResourceReference.Key(scope.getName(), name, + filesystemMatch.getLocale(), filesystemMatch.getStyle(), + filesystemMatch.getVariation()); + + try + { + filesystemMatch.close(); + } + catch (IOException e) + { + log.error("failed to close", e); + } + + if (!urlKey.equals(filesystemKey)) + { + return new ResourceReference.UrlAttributes(filesystemKey.getLocale(), + filesystemKey.getStyle(), filesystemKey.getVariation()); + } + else + { + return urlAttributes; + } + } + } diff --git a/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java b/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java index 83014337b5..324531d990 100644 --- a/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java +++ b/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java @@ -111,6 +111,8 @@ public class PackageResourceReference extends ResourceReference public PackageResource getResource() { final String extension = getExtension(); + final Class<?> scope = getScope(); + final String name = getName(); final PackageResource resource; @@ -122,43 +124,12 @@ public class PackageResourceReference extends ResourceReference //resource attributes (locale, style, variation) might be encoded in the URL final Url url = requestCycle.getRequest().getUrl(); urlAttributes = ResourceUtil.decodeResourceReferenceAttributes(url); + urlAttributes = PackageResource.sanitize(urlAttributes, scope, name); } String currentVariation = getCurrentVariation(urlAttributes); String currentStyle = getCurrentStyle(urlAttributes); Locale currentLocale = getCurrentLocale(urlAttributes); - Class<?> scope = getScope(); - String name = getName(); - - if (urlAttributes != null) // sanitize - { - PackageResource urlResource = new PackageResource(scope, name, currentLocale, - currentStyle, currentVariation); - urlResource.setCachingEnabled(false); - IResourceStream filesystemMatch = urlResource.getResourceStream(); - - ResourceReference.Key urlKey = new ResourceReference.Key(scope.getName(), name, - currentLocale, currentStyle, currentVariation); - - ResourceReference.Key filesystemKey = new ResourceReference.Key(scope.getName(), name, - filesystemMatch.getLocale(), filesystemMatch.getStyle(), - filesystemMatch.getVariation()); - - if (!urlKey.equals(filesystemKey)) - { - currentLocale = filesystemKey.getLocale(); - currentStyle = filesystemKey.getStyle(); - currentVariation = filesystemKey.getVariation(); - } - try - { - filesystemMatch.close(); - } - catch (IOException e) - { - log.error("failed to close", e); - } - } if (CSS_EXTENSION.equals(extension)) {
