You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by pe...@apache.org on 2010/11/25 00:29:23 UTC
svn commit: r1038871 - in /wicket/trunk/wicket/src:
main/java/org/apache/wicket/request/mapper/
test/java/org/apache/wicket/request/mapper/
Author: pete
Date: Wed Nov 24 23:29:23 2010
New Revision: 1038871
URL: http://svn.apache.org/viewvc?rev=1038871&view=rev
Log:
WICKET-3194: looking up last modified timestamps can indeed be slow so now these lookups are cached for the lifetime of a request cycle
Modified:
wicket/trunk/wicket/src/main/java/org/apache/wicket/request/mapper/BasicResourceReferenceMapper.java
wicket/trunk/wicket/src/test/java/org/apache/wicket/request/mapper/AbstractResourceReferenceMapperTest.java
wicket/trunk/wicket/src/test/java/org/apache/wicket/request/mapper/BasicResourceReferenceMapperTest.java
Modified: wicket/trunk/wicket/src/main/java/org/apache/wicket/request/mapper/BasicResourceReferenceMapper.java
URL: http://svn.apache.org/viewvc/wicket/trunk/wicket/src/main/java/org/apache/wicket/request/mapper/BasicResourceReferenceMapper.java?rev=1038871&r1=1038870&r2=1038871&view=diff
==============================================================================
--- wicket/trunk/wicket/src/main/java/org/apache/wicket/request/mapper/BasicResourceReferenceMapper.java (original)
+++ wicket/trunk/wicket/src/main/java/org/apache/wicket/request/mapper/BasicResourceReferenceMapper.java Wed Nov 24 23:29:23 2010
@@ -16,18 +16,24 @@
*/
package org.apache.wicket.request.mapper;
+import org.apache.wicket.MetaDataKey;
+import org.apache.wicket.ThreadContext;
import org.apache.wicket.request.IRequestHandler;
import org.apache.wicket.request.Request;
import org.apache.wicket.request.Url;
+import org.apache.wicket.request.cycle.RequestCycle;
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.ResourceReference;
import org.apache.wicket.util.IProvider;
+import org.apache.wicket.util.lang.Args;
import org.apache.wicket.util.lang.WicketObjects;
import org.apache.wicket.util.time.Time;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import java.util.StringTokenizer;
/**
@@ -48,7 +54,12 @@ import java.util.StringTokenizer;
*/
class BasicResourceReferenceMapper extends AbstractResourceReferenceMapper
{
- private static final String TIMESTAMP_PREFIX = "-ts";
+ // timestamp cache stored in request cycle meta data
+ protected static final MetaDataKey<Map<ResourceReference, Time>> TIMESTAMP_KEY =
+ new MetaDataKey<Map<ResourceReference, Time>>() {};
+
+ protected static final String TIMESTAMP_PREFIX = "-ts";
+
private final IPageParametersEncoder pageParametersEncoder;
// if true, timestamps should be added to resource names
@@ -58,7 +69,7 @@ class BasicResourceReferenceMapper exten
* Construct.
*
* @param pageParametersEncoder
- * @param relativePathPartEscapeSequence
+ * @param timestamps
*/
public BasicResourceReferenceMapper(IPageParametersEncoder pageParametersEncoder, IProvider<Boolean> timestamps)
{
@@ -177,7 +188,7 @@ class BasicResourceReferenceMapper exten
}
/**
- * @see org.apache.wicket.request.IRequestMapper#mapHandler(org.apache.org.apache.wicket.request.IRequestHandler)
+ * @see org.apache.wicket.request.IRequestMapper#mapHandler(org.apache.wicket.request.IRequestHandler)
*/
public Url mapHandler(IRequestHandler requestHandler)
{
@@ -202,8 +213,8 @@ class BasicResourceReferenceMapper exten
// on the last component of the resource path add the timestamp
if (isTimestampsEnabled() && tokens.hasMoreTokens() == false)
{
- // get last modification of resource
- Time lastModified = reference.getLastModified();
+ // get last modification of resource (cached during the current request cycle)
+ Time lastModified = getLastModifiedTimestampUsingCache(reference);
// if resource provides a timestamp we include it in resource name
if (lastModified != null)
@@ -245,6 +256,81 @@ class BasicResourceReferenceMapper exten
}
/**
+ * That method gets the last modification timestamp from the specified resource reference.
+ * <p/>
+ * The timestamp is cached in the meta data of the current request cycle to
+ * eliminate repeated lookups of the same resource reference
+ * which will harm performance.
+ *
+ * @param resourceReference
+ * resource reference
+ *
+ * @return last modification timestamp or <code>null</code> if no timestamp provided
+ */
+ protected Time getLastModifiedTimestampUsingCache(ResourceReference resourceReference)
+ {
+ // try to lookup current request cycle
+ RequestCycle requestCycle = ThreadContext.getRequestCycle();
+
+ // no request cycle: this should not happen unless we e.g. run a plain test case without WicketTester
+ if (requestCycle == null)
+ return resourceReference.getLastModified();
+
+ // retrieve cache from current request cycle
+ Map<ResourceReference, Time> cache = requestCycle.getMetaData(TIMESTAMP_KEY);
+
+ // create it on first call
+ if (cache == null)
+ {
+ cache = new HashMap<ResourceReference, Time>();
+ requestCycle.setMetaData(TIMESTAMP_KEY, cache);
+ }
+
+ final Time lastModified;
+
+ // lookup timestamp from cache (may contain NULL values which are valid)
+ if (cache.containsKey(resourceReference))
+ {
+ lastModified = cache.get(resourceReference);
+ }
+ else
+ {
+ // otherwise retrieve timestamp from resource
+ lastModified = resourceReference.getLastModified();
+
+ // and put it in cache
+ cache.put(resourceReference, lastModified);
+ }
+ return lastModified;
+ }
+
+ /**
+ * Remove a timestamp cache entry for a resource reference from the <b>current</b> request cycle
+ * <p/>
+ * Can't even imagine a valid situation but in case someone really needs it the method is there!
+ */
+ public static void removeLastModifiedTimestampFromCache(ResourceReference resourceReference)
+ {
+ Args.notNull(resourceReference, "resourceReference");
+
+ // lookup current request cycle
+ RequestCycle cycle = RequestCycle.get();
+
+ if(cycle != null)
+ {
+ // retrieve cache
+ Map<ResourceReference, Time> cache = cycle.getMetaData(TIMESTAMP_KEY);
+
+ // if there is a cache
+ if (cache != null)
+ {
+ // remove the resource entry
+ cache.remove(resourceReference);
+ }
+ }
+ }
+
+ /**
* @see org.apache.wicket.request.IRequestMapper#getCompatibilityScore(org.apache.wicket.request.Request)
*/
public int getCompatibilityScore(Request request)
Modified: wicket/trunk/wicket/src/test/java/org/apache/wicket/request/mapper/AbstractResourceReferenceMapperTest.java
URL: http://svn.apache.org/viewvc/wicket/trunk/wicket/src/test/java/org/apache/wicket/request/mapper/AbstractResourceReferenceMapperTest.java?rev=1038871&r1=1038870&r2=1038871&view=diff
==============================================================================
--- wicket/trunk/wicket/src/test/java/org/apache/wicket/request/mapper/AbstractResourceReferenceMapperTest.java (original)
+++ wicket/trunk/wicket/src/test/java/org/apache/wicket/request/mapper/AbstractResourceReferenceMapperTest.java Wed Nov 24 23:29:23 2010
@@ -16,16 +16,19 @@
*/
package org.apache.wicket.request.mapper;
+import java.io.Serializable;
import java.util.Locale;
import org.apache.wicket.request.resource.IResource;
import org.apache.wicket.request.resource.ResourceReference;
+import org.apache.wicket.util.time.Time;
/**
* @author Matej Knopp
*/
-public abstract class AbstractResourceReferenceMapperTest extends AbstractMapperTest
+public abstract class AbstractResourceReferenceMapperTest extends AbstractMapperTest implements Serializable
{
+ public static final Time LAST_MODIFIED = Time.milliseconds(12345678L);
/**
* Construct.
@@ -99,7 +102,7 @@ public abstract class AbstractResourceRe
public IResource getResource()
{
return resource1;
- };
+ }
};
protected ResourceReference reference1_a = new ResourceReference(
@@ -111,7 +114,7 @@ public abstract class AbstractResourceRe
public IResource getResource()
{
return resource1;
- };
+ }
};
protected ResourceReference reference1_b = new ResourceReference(
@@ -123,7 +126,7 @@ public abstract class AbstractResourceRe
public IResource getResource()
{
return resource1;
- };
+ }
};
protected ResourceReference reference2 = new ResourceReference(
@@ -136,7 +139,7 @@ public abstract class AbstractResourceRe
public IResource getResource()
{
return resource2;
- };
+ }
};
protected ResourceReference reference2_a = new ResourceReference(
@@ -149,7 +152,7 @@ public abstract class AbstractResourceRe
public IResource getResource()
{
return resource2;
- };
+ }
};
protected ResourceReference reference3 = new ResourceReference(
@@ -161,7 +164,7 @@ public abstract class AbstractResourceRe
public IResource getResource()
{
return resource3;
- };
+ }
};
protected ResourceReference reference4 = new ResourceReference(
@@ -173,7 +176,7 @@ public abstract class AbstractResourceRe
public IResource getResource()
{
return resource4;
- };
+ }
};
protected ResourceReference reference5 = new ResourceReference(
@@ -185,7 +188,7 @@ public abstract class AbstractResourceRe
public IResource getResource()
{
return resource5;
- };
+ }
};
protected ResourceReference reference6 = new ResourceReference(
@@ -198,7 +201,7 @@ public abstract class AbstractResourceRe
public IResource getResource()
{
return resource6;
- };
+ }
};
@Override
@@ -216,4 +219,30 @@ public abstract class AbstractResourceRe
context.getResourceReferenceRegistry().registerResourceReference(reference5);
context.getResourceReferenceRegistry().registerResourceReference(reference6);
}
+
+ // resource reference that monitors and supports the last modified timestamp
+ protected class ResourceReferenceWithTimestamp extends ResourceReference
+ {
+ protected int lastModifiedInvocationCount = 0;
+ private final Time lastModified;
+
+ public ResourceReferenceWithTimestamp(Time lastModified)
+ {
+ super(AbstractResourceReferenceMapperTest.class, "timestamped", Locale.ENGLISH, "style", null);
+ this.lastModified = lastModified;
+ }
+
+ @Override
+ public IResource getResource()
+ {
+ return resource4;
+ }
+
+ @Override
+ public Time getLastModified()
+ {
+ lastModifiedInvocationCount++;
+ return lastModified;
+ }
+ }
}
Modified: wicket/trunk/wicket/src/test/java/org/apache/wicket/request/mapper/BasicResourceReferenceMapperTest.java
URL: http://svn.apache.org/viewvc/wicket/trunk/wicket/src/test/java/org/apache/wicket/request/mapper/BasicResourceReferenceMapperTest.java?rev=1038871&r1=1038870&r2=1038871&view=diff
==============================================================================
--- wicket/trunk/wicket/src/test/java/org/apache/wicket/request/mapper/BasicResourceReferenceMapperTest.java (original)
+++ wicket/trunk/wicket/src/test/java/org/apache/wicket/request/mapper/BasicResourceReferenceMapperTest.java Wed Nov 24 23:29:23 2010
@@ -18,19 +18,24 @@ package org.apache.wicket.request.mapper
import java.util.Locale;
+import org.apache.wicket.ThreadContext;
import org.apache.wicket.request.IRequestHandler;
import org.apache.wicket.request.Url;
+import org.apache.wicket.request.cycle.RequestCycle;
import org.apache.wicket.request.handler.resource.ResourceReferenceRequestHandler;
import org.apache.wicket.request.mapper.parameter.PageParameters;
import org.apache.wicket.request.mapper.parameter.PageParametersEncoder;
import org.apache.wicket.util.ValueProvider;
+import org.apache.wicket.util.time.Time;
+import org.mockito.Mockito;
/**
* @author Matej Knopp
*/
public class BasicResourceReferenceMapperTest extends AbstractResourceReferenceMapperTest
{
- private static final ValueProvider<Boolean> TIMESTAMPS = new ValueProvider<Boolean>(false);
+ private static final ValueProvider<Boolean> TIMESTAMPS_OFF = new ValueProvider<Boolean>(false);
+ private static final ValueProvider<Boolean> TIMESTAMPS_ON = new ValueProvider<Boolean>(true);
/**
* Construct.
@@ -39,7 +44,18 @@ public class BasicResourceReferenceMappe
{
}
- private final BasicResourceReferenceMapper encoder = new BasicResourceReferenceMapper(new PageParametersEncoder(), TIMESTAMPS)
+ private final BasicResourceReferenceMapper encoder =
+ new BasicResourceReferenceMapper(new PageParametersEncoder(), TIMESTAMPS_OFF)
+ {
+ @Override
+ protected IMapperContext getContext()
+ {
+ return context;
+ }
+ };
+
+ private final BasicResourceReferenceMapper encoderWithTimestamps =
+ new BasicResourceReferenceMapper(new PageParametersEncoder(), TIMESTAMPS_ON)
{
@Override
protected IMapperContext getContext()
@@ -405,4 +421,58 @@ public class BasicResourceReferenceMappe
assertEquals("wicket/resource/" + CLASS_NAME + "/reference4?en-style&p1=v1&p2=v2",
url.toString());
}
+
+ public void testLastModifiedTimestampIsPartOfUrl()
+ {
+ long millis = 12345678L;
+ final ResourceReferenceWithTimestamp reference = new ResourceReferenceWithTimestamp(Time.milliseconds(millis));
+ final IRequestHandler handler = new ResourceReferenceRequestHandler(reference, null);
+
+ // request url with timestamp
+ Url url = encoderWithTimestamps.mapHandler(handler);
+
+ // check that url contains timestamp
+ String timestampPart = BasicResourceReferenceMapper.TIMESTAMP_PREFIX + Long.toString(millis) + "?";
+ assertTrue(url.toString().contains(timestampPart));
+ }
+
+ @SuppressWarnings({"unchecked"})
+ public void testLastModifiedTimestampCache()
+ {
+ long millis = 87654321L;
+ final ResourceReferenceWithTimestamp reference = new ResourceReferenceWithTimestamp(Time.milliseconds(millis));
+ final IRequestHandler handler = new ResourceReferenceRequestHandler(reference, null);
+
+ // setup mock request cycle
+ RequestCycle cycle = Mockito.mock(RequestCycle.class);
+ ThreadContext.setRequestCycle(cycle);
+
+ // request url with timestamp
+ Url url1 = encoderWithTimestamps.mapHandler(handler);
+ assertNotNull(url1);
+ assertEquals(1, reference.lastModifiedInvocationCount);
+
+ // subsequent request should take timestamp from request cycle scoped cache
+ Url url2 = encoderWithTimestamps.mapHandler(handler);
+ assertNotNull(url2);
+
+ Url url3 = encoderWithTimestamps.mapHandler(handler);
+ assertNotNull(url3);
+
+ assertEquals(1, reference.lastModifiedInvocationCount);
+
+ // urls should be equal
+ assertEquals(url1, url2);
+ assertEquals(url1, url3);
+
+ // clear cache
+ BasicResourceReferenceMapper.removeLastModifiedTimestampFromCache(reference);
+
+ // request url with timestamp (will force a lookup)
+ Url url4 = encoderWithTimestamps.mapHandler(handler);
+ assertNotNull(url4);
+ assertEquals(2, reference.lastModifiedInvocationCount);
+
+ assertEquals(url1, url4);
+ }
}