Skip to content

Commit

Permalink
WICKET-7024 squash to the last commit (WIP)
Browse files Browse the repository at this point in the history
- test cases
- code refactoring
- WIP: the fix works only to mounted resources, next step is to make it
  work with the BasicResourceReferenceMapper
  • Loading branch information
pedrosans authored and bitstorm committed Nov 1, 2024
1 parent 38cb92c commit 755a63a
Show file tree
Hide file tree
Showing 8 changed files with 142 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;

/**
Expand All @@ -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" };
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}

/**
Expand All @@ -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);
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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)
*
Expand Down Expand Up @@ -531,7 +560,7 @@ private ResourceResponse sendResourceError(ResourceResponse resourceResponse, in
@Override
public IResourceStream getResourceStream()
{
return internalGetResourceStream(getCurrentStyle(), getCurrentLocale(), isCachingEnabled());
return internalGetResourceStream(getCurrentStyle(), getCurrentLocale(), isServerResourceStreamReferenceCacheUpdate());
}

/**
Expand All @@ -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)
Expand Down Expand Up @@ -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;
}
}

}
Loading

0 comments on commit 755a63a

Please sign in to comment.