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

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.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 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.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 @@
 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 @@ void noRequestCycle()
 	}
 
 	@Test
-	public void getResouceWithNoStyle()
+	public void getResourceWithNoStyle()
 	{
 		tester.executeUrl(
 			"wicket/resource/org.apache.wicket.core.request.resource.PackageResourceReferenceTest/a.css");
@@ -435,6 +443,48 @@ public void decodeStyleFromUrl()
 		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
 	 */

wicket-core-tests/src/test/java/org/apache/wicket/core/util/resource/locator/CachingResourceStreamLocatorTest.java

@@ -168,7 +168,7 @@ void fileResource()
 		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 @@ void fileResource()
 
 		// 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 @@ void fileResourceDifferentExtensions()
 		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 @@ void fileResourceDifferentExtensions()
 
 		// 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 @@ void lightweightResource()
 		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 @@ void lightweightResource()
 		// 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);
 	}
 }

wicket-core/src/main/java/org/apache/wicket/core/request/mapper/BasicResourceReferenceMapper.java

@@ -27,10 +27,7 @@
 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 IRequestHandler mapRequest(Request request)
 
 			Class<?> scope = resolveClass(className);
 
+			// attributes = PackageResource.sanitize(attributes, scope, name.toString());
+
 			if (scope != null && scope.getPackage() != null)
 			{
 				ResourceReference res = getContext().getResourceReferenceRegistry()

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,

wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/ResourceStreamLocator.java

@@ -140,25 +140,10 @@ public IResourceStream locate(final Class<?> clazz, final String path)
 	 * @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);

wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/caching/CachingResourceStreamLocator.java

@@ -119,7 +119,6 @@ public IResourceStream locate(Class<?> scope, String path, String style, String
 		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 IResourceStream locate(Class<?> scope, String path, String style, String
 		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)
 			{

wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResource.java

@@ -32,6 +32,7 @@
 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 PackageResourceBlockedException(String message)
 	 */
 	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 void setCachingEnabled(final boolean enabled)
 		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 @@ private ResourceResponse sendResourceError(ResourceResponse resourceResponse, in
 	@Override
 	public IResourceStream getResourceStream()
 	{
-		return internalGetResourceStream(getCurrentStyle(), getCurrentLocale(), isCachingEnabled());
+		return internalGetResourceStream(getCurrentStyle(), getCurrentLocale(), isServerResourceStreamReferenceCacheUpdate());
 	}
 
 	/**
@@ -557,8 +586,18 @@ private IResourceStream internalGetResourceStream(final String style, final Loca
 		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 PackageResource readBuffered(boolean readBuffered)
 		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;
+		}
+	}
+
 }