You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by jk...@apache.org on 2006/11/01 04:58:32 UTC
svn commit: r469774 - in /tapestry/tapestry4/trunk: ./
tapestry-annotations/src/test/org/apache/tapestry/annotations/
tapestry-framework/src/java/org/apache/tapestry/pageload/
tapestry-framework/src/java/org/apache/tapestry/resolver/
tapestry-framework...
Author: jkuhnert
Date: Tue Oct 31 19:58:31 2006
New Revision: 469774
URL: http://svn.apache.org/viewvc?view=rev&rev=469774
Log:
Fixes for TAPESTRY-846. Don't trust this processor so will also run on more capable machine to be sure.
Added:
tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/pageload/PageSourceTest.java
Modified:
tapestry/tapestry4/trunk/pom.xml
tapestry/tapestry4/trunk/tapestry-annotations/src/test/org/apache/tapestry/annotations/InitialValueAnnotationWorkerTest.java
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageLoader.java
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageSource.java
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/resolver/PageSpecificationResolverImpl.java
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/services/impl/ComponentConstructorFactoryImpl.java
tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/services/impl/ObjectPoolImpl.java
tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/data/TestNullDataSqueezerFilter.java
tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/enhance/TestAutowireWorker.java
tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/enhance/autowire-multiple.xml
tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/resolver/PageSpecificationResolverTest.java
Modified: tapestry/tapestry4/trunk/pom.xml
URL: http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/pom.xml?view=diff&rev=469774&r1=469773&r2=469774
==============================================================================
--- tapestry/tapestry4/trunk/pom.xml (original)
+++ tapestry/tapestry4/trunk/pom.xml Tue Oct 31 19:58:31 2006
@@ -208,6 +208,11 @@
</exclusion>
</exclusions>
</dependency>
+ <dependency>
+ <groupId>backport-util-concurrent</groupId>
+ <artifactId>backport-util-concurrent</artifactId>
+ <version>2.2</version>
+ </dependency>
</dependencies>
</dependencyManagement>
@@ -226,6 +231,7 @@
<value>en_US</value>
</property>
</systemProperties>
+ <parallel>false</parallel>
</configuration>
</plugin>
<plugin>
Modified: tapestry/tapestry4/trunk/tapestry-annotations/src/test/org/apache/tapestry/annotations/InitialValueAnnotationWorkerTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-annotations/src/test/org/apache/tapestry/annotations/InitialValueAnnotationWorkerTest.java?view=diff&rev=469774&r1=469773&r2=469774
==============================================================================
--- tapestry/tapestry4/trunk/tapestry-annotations/src/test/org/apache/tapestry/annotations/InitialValueAnnotationWorkerTest.java (original)
+++ tapestry/tapestry4/trunk/tapestry-annotations/src/test/org/apache/tapestry/annotations/InitialValueAnnotationWorkerTest.java Tue Oct 31 19:58:31 2006
@@ -35,7 +35,7 @@
public void testCanEnhance()
{
InitialValueAnnotationWorker worker = new InitialValueAnnotationWorker();
-
+
assertEquals(false, worker.canEnhance(findMethod(AnnotatedPage.class, "getMapBean")));
assertEquals(true, worker.canEnhance(findMethod(
AnnotatedPage.class,
Modified: tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageLoader.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageLoader.java?view=diff&rev=469774&r1=469773&r2=469774
==============================================================================
--- tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageLoader.java (original)
+++ tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageLoader.java Tue Oct 31 19:58:31 2006
@@ -604,10 +604,11 @@
// Walk through the complete component tree to set up the default
// parameter values.
+
_establishDefaultParameterValuesWalker.walkComponentTree(page);
-
+
establishInheritedBindings();
-
+
// Walk through the complete component tree to ensure that required
// parameters are bound
_verifyRequiredParametersWalker.walkComponentTree(page);
Modified: tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageSource.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageSource.java?view=diff&rev=469774&r1=469773&r2=469774
==============================================================================
--- tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageSource.java (original)
+++ tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/pageload/PageSource.java Tue Oct 31 19:58:31 2006
@@ -14,6 +14,10 @@
package org.apache.tapestry.pageload;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.locks.ReentrantLock;
+
import org.apache.hivemind.ClassResolver;
import org.apache.tapestry.IEngine;
import org.apache.tapestry.IPage;
@@ -42,6 +46,8 @@
public class PageSource implements IPageSource
{
+ private static final int ESTIMATED_PAGES = 45;
+
/** set by container. */
private ClassResolver _classResolver;
@@ -58,7 +64,14 @@
*/
private ObjectPool _pool;
-
+
+ /**
+ * Provides concurrent access to page keys to prevent
+ * possibly creating more than one page spec of the same page.
+ */
+
+ private ConcurrentMap _keyMap = new ConcurrentHashMap(ESTIMATED_PAGES);
+
public ClassResolver getClassResolver()
{
return _classResolver;
@@ -70,13 +83,10 @@
protected MultiKey buildKey(IEngine engine, String pageName)
{
- Object[] keys;
-
- keys = new Object[]
- { pageName, engine.getLocale() };
-
+ Object[] keys = new Object[] { pageName, engine.getLocale() };
+
// Don't make a copy, this array is just for the MultiKey.
-
+
return new MultiKey(keys, false);
}
@@ -87,17 +97,37 @@
protected MultiKey buildKey(IPage page)
{
- Object[] keys;
-
- keys = new Object[]
- { page.getPageName(), page.getLocale() };
-
+ Object[] keys = new Object[] { page.getPageName(), page.getLocale() };
+
// Don't make a copy, this array is just for the MultiKey.
return new MultiKey(keys, false);
}
/**
+ * Gets a simple lock for the given page that should be used before
+ * doing any page specification resolution operations.
+ *
+ * @param key The page key to use.
+ * @return The existing/created lock for this specific page.
+ */
+ protected ReentrantLock getPageLock(Object key)
+ {
+ ReentrantLock lock = (ReentrantLock)_keyMap.get(key);
+
+ if (lock != null)
+ return lock;
+
+ lock = new ReentrantLock();
+
+ // writes are synchronized, this is where the "magic" happens
+
+ _keyMap.put(key, lock);
+
+ return lock;
+ }
+
+ /**
* Gets the page from a pool, or otherwise loads the page. This operation is threadsafe.
*/
@@ -105,29 +135,48 @@
{
IEngine engine = cycle.getEngine();
Object key = buildKey(engine, pageName);
- IPage result = (IPage) _pool.get(key);
-
- if (result == null)
- {
- _pageSpecificationResolver.resolve(cycle, pageName);
-
- // The loader is responsible for invoking attach(),
- // and for firing events to PageAttachListeners
-
- result = _loader.loadPage(
- _pageSpecificationResolver.getSimplePageName(),
- _pageSpecificationResolver.getNamespace(),
- cycle,
- _pageSpecificationResolver.getSpecification());
- }
- else
- {
- // But for pooled pages, we are responsible.
- // This call will also fire events to any PageAttachListeners
-
- result.attach(engine, cycle);
+ ReentrantLock lock = getPageLock(key);
+
+ IPage result = null;
+
+ try {
+ // lock our page specific key lock first
+ // This is only a temporary measure until a more robust
+ // page pool implementation can be created.
+
+ lock.lockInterruptibly();
+
+ result = (IPage) _pool.get(key);
+
+ if (result == null)
+ {
+ _pageSpecificationResolver.resolve(cycle, pageName);
+
+ // The loader is responsible for invoking attach(),
+ // and for firing events to PageAttachListeners
+
+ result = _loader.loadPage(
+ _pageSpecificationResolver.getSimplePageName(),
+ _pageSpecificationResolver.getNamespace(),
+ cycle,
+ _pageSpecificationResolver.getSpecification());
+ }
+ else
+ {
+ // But for pooled pages, we are responsible.
+ // This call will also fire events to any PageAttachListeners
+
+ result.attach(engine, cycle);
+ }
+
+ } catch (InterruptedException e) {
+
+ throw new RuntimeException(e);
+ } finally {
+
+ lock.unlock();
}
-
+
return result;
}
Modified: tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/resolver/PageSpecificationResolverImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/resolver/PageSpecificationResolverImpl.java?view=diff&rev=469774&r1=469773&r2=469774
==============================================================================
--- tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/resolver/PageSpecificationResolverImpl.java (original)
+++ tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/resolver/PageSpecificationResolverImpl.java Tue Oct 31 19:58:31 2006
@@ -255,17 +255,16 @@
// namespace (typically, the application specification). This will be
// used when
// searching for the page's message catalog or other related assets.
-
- Resource pageResource = namespaceLocation
- .getRelativeResource(_simpleName + ".page");
-
+
+ Resource pageResource = namespaceLocation.getRelativeResource(_simpleName + ".page");
+
IComponentSpecification specification = new ComponentSpecification();
specification.setPageSpecification(true);
specification.setSpecificationLocation(pageResource);
specification.setLocation(new LocationImpl(resource));
-
+
setSpecification(specification);
-
+
install();
}
@@ -285,16 +284,16 @@
return true;
}
-
+
private void install()
{
INamespace namespace = getNamespace();
IComponentSpecification specification = getSpecification();
-
+
if (_log.isDebugEnabled())
_log.debug(ResolverMessages.installingPage(_simpleName, namespace,
specification));
-
+
namespace.installPageSpecification(_simpleName, specification);
}
Modified: tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/services/impl/ComponentConstructorFactoryImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/services/impl/ComponentConstructorFactoryImpl.java?view=diff&rev=469774&r1=469773&r2=469774
==============================================================================
--- tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/services/impl/ComponentConstructorFactoryImpl.java (original)
+++ tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/services/impl/ComponentConstructorFactoryImpl.java Tue Oct 31 19:58:31 2006
@@ -78,37 +78,36 @@
String className)
{
Defense.notNull(specification, "specification");
-
- synchronized (specification)
- {
- ComponentConstructor result = (ComponentConstructor) _cachedConstructors
- .get(specification);
-
- if (result == null)
- {
+
+ synchronized (specification) {
+
+ ComponentConstructor result = (ComponentConstructor) _cachedConstructors.get(specification);
+
+ if (result == null) {
+
Class baseClass = _classResolver.findClass(className);
-
+
EnhancementOperationImpl eo = new EnhancementOperationImpl(_classResolver,
specification, baseClass, _classFactory, _log);
-
+
// Invoking on the chain is the same as invoking on every
// object in the chain (because method performEnhancement() is type void).
-
+
_chain.performEnhancement(eo, specification);
-
+
result = eo.getConstructor();
-
+
// TODO: This should be optional to work around that IBM JVM bug.
-
+
_validator.validate(baseClass, result.getComponentClass(), specification);
-
+
_cachedConstructors.put(specification, result);
}
-
+
return result;
}
}
-
+
public void setClassFactory(ClassFactory classFactory)
{
_classFactory = classFactory;
Modified: tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/services/impl/ObjectPoolImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/services/impl/ObjectPoolImpl.java?view=diff&rev=469774&r1=469773&r2=469774
==============================================================================
--- tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/services/impl/ObjectPoolImpl.java (original)
+++ tapestry/tapestry4/trunk/tapestry-framework/src/java/org/apache/tapestry/services/impl/ObjectPoolImpl.java Tue Oct 31 19:58:31 2006
@@ -44,7 +44,9 @@
* Pool of Lists (of pooled objects), keyed on arbitrary key.
*/
private Map _pool = new HashMap();
-
+
+ // TODO: This synchronized block isn't incredibly efficient
+
public synchronized Object get(Object key)
{
List pooled = (List) _pool.get(key);
Modified: tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/data/TestNullDataSqueezerFilter.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/data/TestNullDataSqueezerFilter.java?view=diff&rev=469774&r1=469773&r2=469774
==============================================================================
--- tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/data/TestNullDataSqueezerFilter.java (original)
+++ tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/data/TestNullDataSqueezerFilter.java Tue Oct 31 19:58:31 2006
@@ -30,6 +30,7 @@
public void testSqueezeNull()
{
final NullDataSqueezerFilter filter = new NullDataSqueezerFilter();
+
assertNull( filter.unsqueeze( filter.squeeze(( Object )null,null), null ) );
}
Modified: tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/enhance/TestAutowireWorker.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/enhance/TestAutowireWorker.java?view=diff&rev=469774&r1=469773&r2=469774
==============================================================================
--- tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/enhance/TestAutowireWorker.java (original)
+++ tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/enhance/TestAutowireWorker.java Tue Oct 31 19:58:31 2006
@@ -37,17 +37,19 @@
private static final String HELLO_SERVICE_PROPERTY = "helloService";
@Test(alwaysRun = true)
- public void testWithNoService() throws Exception
+ public void test_No_Service() throws Exception
{
assertNotAutowired( RegistryBuilder.constructDefaultRegistry() );
}
+
@Test
- public void testWithManyServices() throws Exception
+ public void test_Many_Services() throws Exception
{
assertNotAutowired( buildFrameworkRegistry("autowire-multiple.xml" ) );
}
+
@Test
- public void testWithOneService() throws Exception
+ public void test_One_Service() throws Exception
{
final Registry registry = buildFrameworkRegistry("autowire-single.xml" );
Location l = newLocation();
@@ -99,6 +101,7 @@
replay();
final EnhancementWorker worker = ( EnhancementWorker )registry.getService( "tapestry.enhance.AutowireWorker", EnhancementWorker.class );
+
worker.performEnhancement( op, null);
verify();
Modified: tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/enhance/autowire-multiple.xml
URL: http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/enhance/autowire-multiple.xml?view=diff&rev=469774&r1=469773&r2=469774
==============================================================================
--- tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/enhance/autowire-multiple.xml (original)
+++ tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/enhance/autowire-multiple.xml Tue Oct 31 19:58:31 2006
@@ -14,8 +14,8 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
+<module id="autowire.multiple" version="1.0.0">
-<module id="autowire.multiple" version="1.0.0">
<service-point id="Hello1" interface="org.apache.tapestry.enhance.HelloService">
<invoke-factory>
<construct class="org.apache.tapestry.enhance.HelloServiceImpl" />
Added: tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/pageload/PageSourceTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/pageload/PageSourceTest.java?view=auto&rev=469774
==============================================================================
--- tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/pageload/PageSourceTest.java (added)
+++ tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/pageload/PageSourceTest.java Tue Oct 31 19:58:31 2006
@@ -0,0 +1,70 @@
+// Copyright 2004, 2005 The Apache Software Foundation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package org.apache.tapestry.pageload;
+
+import java.util.concurrent.locks.ReentrantLock;
+
+import org.apache.tapestry.BaseComponentTestCase;
+import org.testng.annotations.AfterSuite;
+import org.testng.annotations.BeforeSuite;
+import org.testng.annotations.Test;
+
+
+/**
+ * Tests functionality of {@link PageSource} implementation, esp
+ * in how it handles threading semantics.
+ */
+public class PageSourceTest extends BaseComponentTestCase
+{
+
+ static final String PAGE_NAME = "TestPage";
+
+ PageSource _pageSource;
+
+ int _callCount = 0;
+
+ @BeforeSuite
+ public void create()
+ {
+ _pageSource = new PageSource();
+ }
+
+ /**
+ * No matter how many threads are used the _callCount
+ * member should always equal the invocationCount after the test
+ * is done running.
+ *
+ * @throws InterruptedException
+ */
+ @Test(invocationCount = 80, threadPoolSize = 3)
+ public void test_Page_Lock()
+ throws InterruptedException
+ {
+ ReentrantLock lock = _pageSource.getPageLock(PAGE_NAME);
+
+ try {
+ lock.lockInterruptibly();
+
+ _callCount++;
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ @AfterSuite
+ public void check_Counts()
+ {
+ assertEquals(_callCount, 80);
+ }
+}
Modified: tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/resolver/PageSpecificationResolverTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/resolver/PageSpecificationResolverTest.java?view=diff&rev=469774&r1=469773&r2=469774
==============================================================================
--- tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/resolver/PageSpecificationResolverTest.java (original)
+++ tapestry/tapestry4/trunk/tapestry-framework/src/test/org/apache/tapestry/resolver/PageSpecificationResolverTest.java Tue Oct 31 19:58:31 2006
@@ -43,7 +43,7 @@
@Test
public class PageSpecificationResolverTest extends AbstractSpecificationResolverTestCase
{
- private static class MockApplicationNamespace implements INamespace
+ public static class MockApplicationNamespace implements INamespace
{
String _pageName;
@@ -153,12 +153,12 @@
}
- private ISpecificationResolverDelegate newDelegate()
+ protected ISpecificationResolverDelegate newDelegate()
{
return newMock(ISpecificationResolverDelegate.class);
}
- private INamespace newNamespace(String pageName, IComponentSpecification spec)
+ protected INamespace newNamespace(String pageName, IComponentSpecification spec)
{
INamespace namespace = newNamespace();
checkOrder(namespace, false);
@@ -171,7 +171,7 @@
return namespace;
}
- private ComponentPropertySource newPropertySource(INamespace namespace)
+ protected ComponentPropertySource newPropertySource(INamespace namespace)
{
ComponentPropertySource source = newMock(ComponentPropertySource.class);
@@ -181,7 +181,7 @@
return source;
}
- private ISpecificationSource newSource(INamespace application, INamespace framework)
+ protected ISpecificationSource newSource(INamespace application, INamespace framework)
{
ISpecificationSource source = newSource();
@@ -192,7 +192,7 @@
return source;
}
- private ISpecificationSource newSource(INamespace application, INamespace framework,
+ protected ISpecificationSource newSource(INamespace application, INamespace framework,
Resource resource, IComponentSpecification pageSpec)
{
ISpecificationSource source = newSource();
@@ -850,7 +850,7 @@
verify();
}
-
+
private void trainGetPageSpecification(INamespace namespace, String pageName,
IComponentSpecification spec)
{
@@ -862,4 +862,4 @@
{
expect(source.getPageSpecification(resource)).andReturn(pageSpec);
}
-}
\ No newline at end of file
+}