Repository: wicket Updated Branches: refs/heads/wicket-6.x f66d4b810 -> 8b5a4aab7
WICKET-5968 CachingResourceLocator lookup key doesn't take strict into account Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/8b5a4aab Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/8b5a4aab Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/8b5a4aab Branch: refs/heads/wicket-6.x Commit: 8b5a4aab78a2826c7b3ef558566be04f73632190 Parents: f66d4b8 Author: Martijn Dashorst <[email protected]> Authored: Tue Aug 11 10:11:28 2015 +0200 Committer: Martijn Dashorst <[email protected]> Committed: Tue Aug 11 15:09:42 2015 +0200 ---------------------------------------------------------------------- .../caching/CachingResourceStreamLocator.java | 49 ++++--- .../CachingResourceStreamLocatorTest.java | 130 +++++++++++++++++-- 2 files changed, 149 insertions(+), 30 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/8b5a4aab/wicket-core/src/main/java/org/apache/wicket/core/util/resource/locator/caching/CachingResourceStreamLocator.java ---------------------------------------------------------------------- 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 de2ec65..9e2a57b 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 @@ -73,7 +73,7 @@ public class CachingResourceStreamLocator implements IResourceStreamLocator @Override public IResourceStream locate(Class<?> clazz, String path) { - CacheKey key = new CacheKey(clazz.getName(), path, null, null, null, null); + CacheKey key = new CacheKey(clazz.getName(), path, null, null, null, null, true); IResourceStreamReference resourceStreamReference = cache.get(key); final IResourceStream result; @@ -113,7 +113,7 @@ public class CachingResourceStreamLocator implements IResourceStreamLocator public IResourceStream locate(Class<?> scope, String path, String style, String variation, Locale locale, String extension, boolean strict) { - CacheKey key = new CacheKey(scope.getName(), path, extension, locale, style, variation); + CacheKey key = new CacheKey(scope.getName(), path, extension, locale, style, variation, strict); IResourceStreamReference resourceStreamReference = cache.get(key); final IResourceStream result; @@ -149,37 +149,54 @@ public class CachingResourceStreamLocator implements IResourceStreamLocator */ private static class CacheKey extends Key { + private static final long serialVersionUID = 1L; + /** * The file extension */ private final String extension; - private CacheKey(String scope, String name, String extension, Locale locale, String style, String variation) + /** Whether the key was looked up using a strict matching search */ + private final boolean strict; + + private CacheKey(String scope, String name, String extension, Locale locale, String style, String variation, boolean strict) { super(scope, name, locale, style, variation); this.extension = extension; + this.strict = strict; } @Override - public boolean equals(Object o) + public int hashCode() { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - if (!super.equals(o)) return false; - - CacheKey cacheKey = (CacheKey) o; - - return !(extension != null ? !extension.equals(cacheKey.extension) : cacheKey.extension != null); - + final int prime = 31; + int result = super.hashCode(); + result = prime * result + ((extension == null) ? 0 : extension.hashCode()); + result = prime * result + (strict ? 1231 : 1237); + return result; } @Override - public int hashCode() + public boolean equals(Object obj) { - int result = super.hashCode(); - result = 31 * result + (extension != null ? extension.hashCode() : 0); - return result; + if (this == obj) + return true; + if (!super.equals(obj)) + return false; + if (getClass() != obj.getClass()) + return false; + CacheKey other = (CacheKey)obj; + if (extension == null) + { + if (other.extension != null) + return false; + } + else if (!extension.equals(other.extension)) + return false; + if (strict != other.strict) + return false; + return true; } } } http://git-wip-us.apache.org/repos/asf/wicket/blob/8b5a4aab/wicket-core/src/test/java/org/apache/wicket/util/resource/locator/CachingResourceStreamLocatorTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/util/resource/locator/CachingResourceStreamLocatorTest.java b/wicket-core/src/test/java/org/apache/wicket/util/resource/locator/CachingResourceStreamLocatorTest.java index 65ea0ad..ca01230 100644 --- a/wicket-core/src/test/java/org/apache/wicket/util/resource/locator/CachingResourceStreamLocatorTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/util/resource/locator/CachingResourceStreamLocatorTest.java @@ -16,6 +16,10 @@ */ package org.apache.wicket.util.resource.locator; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.junit.Assert.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -23,12 +27,18 @@ import static org.mockito.Mockito.when; import java.io.File; import java.net.URL; +import java.util.Locale; +import org.apache.wicket.ajax.AbstractDefaultAjaxBehavior; +import org.apache.wicket.core.util.resource.ClassPathResourceFinder; +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.ResourceStreamLocator; +import org.apache.wicket.core.util.resource.locator.caching.CachingResourceStreamLocator; 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.core.util.resource.UrlResourceStream; -import org.apache.wicket.core.util.resource.locator.caching.CachingResourceStreamLocator; +import org.junit.Assert; import org.junit.Test; /** @@ -60,6 +70,99 @@ public class CachingResourceStreamLocatorTest } /** + * Tests strict before non-strict matching without a specific locale. + */ + @Test + public void strictBeforeNonStrictMatchingWithoutLocaleDoesntResultInInvalidNonStrictMatch() + { + IResourceStreamLocator resourceStreamLocator = new ResourceStreamLocator( + new ClassPathResourceFinder("")); + CachingResourceStreamLocator cachingLocator = new CachingResourceStreamLocator( + resourceStreamLocator); + + String style = null; + String variation = null; + Locale locale = null; + String extension = null; + + String filename = "org/apache/wicket/ajax/res/js/wicket-ajax-jquery-debug.js"; + + // a strict lookup for the resource with no specific locale results in a match + IResourceStream strictLocate = cachingLocator.locate(AbstractDefaultAjaxBehavior.class, + filename, style, variation, locale, extension, true); + + assertThat(strictLocate, is(not(nullValue()))); + + // followed by a non-strict search for the same resource also finds it + IResourceStream nonStrictLocate = cachingLocator.locate(AbstractDefaultAjaxBehavior.class, + filename, style, variation, locale, extension, false); + + assertThat(nonStrictLocate, is(not(nullValue()))); + } + + /** + * Tests strict before non-strict matching with a specific locale. + */ + @Test + public void strictMatchingDoesntInvalidateNonStrictMatching() + { + IResourceStreamLocator resourceStreamLocator = new ResourceStreamLocator( + new ClassPathResourceFinder("")); + CachingResourceStreamLocator cachingLocator = new CachingResourceStreamLocator( + resourceStreamLocator); + + String style = null; + String variation = null; + Locale locale = new Locale("nl", "NL"); + String extension = null; + + String filename = "org/apache/wicket/ajax/res/js/wicket-ajax-jquery-debug.js"; + + // a strict lookup of a localized resource should not find the non-localized resource + IResourceStream strictLocate = cachingLocator.locate(AbstractDefaultAjaxBehavior.class, + filename, style, variation, locale, extension, true); + assertThat(strictLocate, is(nullValue())); + + // but a non-strict lookup with a locale should fall back to the non-localized resource + IResourceStream nonStrictLocate = cachingLocator.locate(AbstractDefaultAjaxBehavior.class, + filename, style, variation, locale, extension, false); + + assertThat(nonStrictLocate, is(not(nullValue()))); + } + + /** + * Tests non-strict before strict matching with a specific locale. + */ + @Test + public void nonStrictMatchingDoesntResultInInvalidStrictMatch() + { + IResourceStreamLocator resourceStreamLocator = new ResourceStreamLocator( + new ClassPathResourceFinder("")); + CachingResourceStreamLocator cachingLocator = new CachingResourceStreamLocator( + resourceStreamLocator); + + String style = null; + String variation = null; + Locale locale = new Locale("nl", "NL"); + String extension = null; + + String filename = "org/apache/wicket/ajax/res/js/wicket-ajax-jquery-debug.js"; + + // a non-strict lookup with a specific locale should find the non-localized resource + IResourceStream nonStrictLocate = cachingLocator.locate(AbstractDefaultAjaxBehavior.class, + filename, style, variation, locale, extension, false); + + assertThat(nonStrictLocate, is(not(nullValue()))); + + // but a strict lookup with a specific locale should not fall back to the non-localized + // resource + IResourceStream strictLocate = cachingLocator.locate(AbstractDefaultAjaxBehavior.class, + filename, style, variation, locale, extension, true); + + assertThat(strictLocate, is(nullValue())); + } + + /** * Tests FileResourceStreamReference */ @Test @@ -69,19 +172,18 @@ public class CachingResourceStreamLocatorTest FileResourceStream frs = new FileResourceStream(new File(".")); - when( - resourceStreamLocator.locate(String.class, "path", "style", "variation", null, - "extension", true)).thenReturn(frs); + when(resourceStreamLocator.locate(String.class, "path", "style", "variation", null, + "extension", true)).thenReturn(frs); CachingResourceStreamLocator cachingLocator = new CachingResourceStreamLocator( - resourceStreamLocator); + resourceStreamLocator); cachingLocator.locate(String.class, "path", "style", "variation", null, "extension", true); cachingLocator.locate(String.class, "path", "style", "variation", null, "extension", true); // 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); + null, "extension", true); } /** @@ -95,9 +197,10 @@ public class CachingResourceStreamLocatorTest FileResourceStream frs = new FileResourceStream(new File(".")); when(resourceStreamLocator.locate(String.class, "path", "style", "variation", null, - "extension", true)).thenReturn(frs); + "extension", true)).thenReturn(frs); - CachingResourceStreamLocator cachingLocator = new CachingResourceStreamLocator(resourceStreamLocator); + CachingResourceStreamLocator cachingLocator = new CachingResourceStreamLocator( + resourceStreamLocator); cachingLocator.locate(String.class, "path", "style", "variation", null, "extension", true); cachingLocator.locate(String.class, "path", "style", "variation", null, "extension", true); @@ -105,9 +208,9 @@ public 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); + null, "extension", true); verify(resourceStreamLocator, times(1)).locate(String.class, "path", "style", "variation", - null, "extension2", true); + null, "extension2", true); } /** @@ -145,9 +248,8 @@ public class CachingResourceStreamLocatorTest StringResourceStream srs = new StringResourceStream("anything"); - when( - resourceStreamLocator.locate(String.class, "path", "style", "variation", null, - "extension", true)).thenReturn(srs); + when(resourceStreamLocator.locate(String.class, "path", "style", "variation", null, + "extension", true)).thenReturn(srs); CachingResourceStreamLocator cachingLocator = new CachingResourceStreamLocator( resourceStreamLocator);
