This is an automated email from the ASF dual-hosted git repository.
thiagohp pushed a commit to branch better-page-invalidation
in repository https://gitbox.apache.org/repos/asf/tapestry-5.git
commit f4cd06bbf5ed7628fcb7ff71444240d976fe3c82
Author: Thiago H. de Paula Figueiredo <thi...@arsmachina.com.br>
AuthorDate: Sun May 7 15:34:13 2023 -0300
TAP5-2742: fixes problems with page classes with subclasses
---
.../internal/plastic/PlasticClassPool.java | 28 ++++++++++++++++
.../services/ComponentInstantiatorSourceImpl.java | 2 +-
.../internal/services/PageSourceImpl.java | 38 ++++++++++++++++------
.../apache/tapestry5/modules/TapestryModule.java | 27 ++++++++++++++-
.../services/pageload/PageClassloaderContext.java | 12 ++++++-
.../pageload/PageClassloaderContextManager.java | 6 ++++
.../PageClassloaderContextManagerImpl.java | 32 ++++++++++++++----
7 files changed, 125 insertions(+), 20 deletions(-)
diff --git
a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
index 1a099843c..9daa47d51 100644
---
a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
+++
b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
@@ -20,6 +20,7 @@ import java.io.InputStream;
import java.lang.annotation.Annotation;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@@ -87,6 +88,8 @@ public class PlasticClassPool implements ClassLoaderDelegate,
Opcodes, PlasticCl
private final List<PlasticClassListener> listeners = new
CopyOnWriteArrayList<PlasticClassListener>();
private PlasticClassPool parent;
+
+ private Collection<PlasticClassPool> children = new ArrayList<>();
private final Cache<String, TypeCategory> typeName2Category = new
Cache<String, TypeCategory>()
{
@@ -517,6 +520,13 @@ public class PlasticClassPool implements
ClassLoaderDelegate, Opcodes, PlasticCl
def = current.baseClassDefs.get(baseClassName);
}
+ // Usually, when df is still null, that's because the superclass
+ // is a page class too
+ if (def == null)
+ {
+ def = findBaseClassDef(baseClassName, current);
+ }
+
assert def != null;
return new PlasticClassImpl(classNode, implementationClassNode,
this, def.inheritanceData, def.staticContext, proxy);
@@ -527,6 +537,23 @@ public class PlasticClassPool implements
ClassLoaderDelegate, Opcodes, PlasticCl
return new PlasticClassImpl(classNode, implementationClassNode, this,
emptyInheritanceData, emptyStaticContext, proxy);
}
+ private BaseClassDef findBaseClassDef(String baseClassName,
PlasticClassPool plasticClassPool)
+ {
+ BaseClassDef def = plasticClassPool.baseClassDefs.get(baseClassName);
+ if (def == null)
+ {
+ for (PlasticClassPool child : plasticClassPool.children)
+ {
+ def = child.findBaseClassDef(baseClassName, child);
+ if (def != null)
+ {
+ break;
+ }
+ }
+ }
+ return def;
+ }
+
/**
* Constructs a class node by reading the raw bytecode for a class and
instantiating a ClassNode
* (via {@link
ClassReader#accept(org.apache.tapestry5.internal.plastic.asm.ClassVisitor,
int)}).
@@ -709,6 +736,7 @@ public class PlasticClassPool implements
ClassLoaderDelegate, Opcodes, PlasticCl
public void setParent(PlasticClassPool parent)
{
this.parent = parent;
+ parent.children.add(this);
}
boolean isEnabled(TransformationOption option)
diff --git
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSourceImpl.java
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSourceImpl.java
index d1b8f258f..351b60200 100644
---
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSourceImpl.java
+++
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSourceImpl.java
@@ -292,7 +292,7 @@ public final class ComponentInstantiatorSourceImpl
implements ComponentInstantia
PlasticProxyFactory proxyFactory =
createPlasticProxyFactory(parent);
rootPageClassloaderContext = new PageClassloaderContext(
- "root", null, Collections.emptySet(), proxyFactory);
+ "root", null, Collections.emptySet(), proxyFactory,
pageClassloaderContextManager::get);
}
else
{
diff --git
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageSourceImpl.java
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageSourceImpl.java
index d19b22708..ff15ab7a7 100644
---
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageSourceImpl.java
+++
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageSourceImpl.java
@@ -113,8 +113,13 @@ public class PageSourceImpl implements PageSource
this.pageClassloaderContextManager = pageClassloaderContextManager;
this.logger = logger;
}
-
+
public Page getPage(String canonicalPageName)
+ {
+ return getPage(canonicalPageName, true);
+ }
+
+ public Page getPage(String canonicalPageName, boolean
invalidateUnknownContext)
{
ComponentResourceSelector selector =
selectorAnalyzer.buildSelectorForRequest();
@@ -134,6 +139,17 @@ public class PageSourceImpl implements PageSource
{
return page;
}
+
+ // Avoiding problems in PlasticClassPool.createTransformation()
+ // when the class being loaded has a page superclass
+ final String className =
componentClassResolver.resolvePageNameToClassName(canonicalPageName);
+ PageClassloaderContext context =
pageClassloaderContextManager.get(className);
+ final Class<?> superclass = getSuperclass(className, context);
+ final String superclassName = superclass.getName();
+ if (componentClassResolver.isPage(superclassName))
+ {
+
getPage(componentClassResolver.resolvePageClassNameToPageName(superclassName),
false);
+ }
// In rare race conditions, we may see the same page loaded
multiple times across
// different threads. The last built one will "evict" the others
from the page cache,
@@ -151,21 +167,23 @@ public class PageSourceImpl implements PageSource
final ComponentPageElement rootElement = page.getRootElement();
componentDependencyRegistry.clear(rootElement);
componentDependencyRegistry.register(rootElement);
- final String className =
componentClassResolver.resolvePageNameToClassName(canonicalPageName);
- final PageClassloaderContext context =
pageClassloaderContextManager.get(className);
- if (context.isUnknown())
- {
- componentDependencyRegistry.disableInvalidations();
-
pageClassloaderContextManager.invalidateAndFireInvalidationEvents(context);
- componentDependencyRegistry.disableInvalidations();
- pageClassloaderContextManager.get(className);
- }
+ context = pageClassloaderContextManager.get(className);
+ pageClassloaderContextManager.get(className);
}
}
}
+ private Class<?> getSuperclass(final String className,
PageClassloaderContext context)
+ {
+ try {
+ return
context.getClassLoader().loadClass(className).getSuperclass();
+ } catch (ClassNotFoundException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
@PostInjection
public void setupInvalidation(@ComponentClasses InvalidationEventHub
classesHub,
@ComponentTemplates InvalidationEventHub
templatesHub,
diff --git
a/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
b/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
index ba60eb924..045c68173 100644
---
a/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
+++
b/tapestry-core/src/main/java/org/apache/tapestry5/modules/TapestryModule.java
@@ -967,7 +967,9 @@ public final class TapestryModule
public void contributeRequestHandler(OrderedConfiguration<RequestFilter>
configuration, Context context,
@Symbol(TapestryHttpSymbolConstants.PRODUCTION_MODE)
- boolean productionMode)
+ boolean productionMode,
+
+ final PageClassloaderContextManager
pageClassloaderContextManager)
{
RequestFilter staticFilesFilter = new StaticFilesFilter(context);
@@ -1010,6 +1012,29 @@ public final class TapestryModule
configuration.add("EndOfRequest", fireEndOfRequestEvent);
configuration.addInstance("ErrorFilter", RequestErrorFilter.class);
+
+ if (!productionMode)
+ {
+
+ // TODO: change this to only invalidate the current page
+ RequestFilter invalidateUnknownContext = new RequestFilter()
+ {
+ public boolean service(Request request, Response response,
RequestHandler handler) throws IOException
+ {
+ try
+ {
+ return handler.service(request, response);
+ } finally
+ {
+
pageClassloaderContextManager.invalidateUnknownContext();
+ }
+ }
+ };
+
+ configuration.add(PageClassloaderContextManager.class.getName(),
+ invalidateUnknownContext, "before:EndOfRequest");
+ }
+
}
/**
diff --git
a/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContext.java
b/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContext.java
index ff6daa952..30b3e5afb 100644
---
a/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContext.java
+++
b/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContext.java
@@ -18,6 +18,7 @@ import java.util.Collections;
import java.util.HashSet;
import java.util.Objects;
import java.util.Set;
+import java.util.function.Function;
import org.apache.tapestry5.commons.services.PlasticProxyFactory;
import org.apache.tapestry5.internal.plastic.PlasticClassLoader;
@@ -50,6 +51,8 @@ public class PageClassloaderContext
private final PlasticProxyFactory proxyFactory;
private PageClassloaderContext root;
+
+ private final Function<String, PageClassloaderContext> provider;
/**
* Name of the <code>unknown</code> context (i.e. the one for controlled
classes
@@ -60,7 +63,8 @@ public class PageClassloaderContext
public PageClassloaderContext(String name,
PageClassloaderContext parent,
Set<String> classNames,
- PlasticProxyFactory plasticProxyFactory)
+ PlasticProxyFactory plasticProxyFactory,
+ Function<String, PageClassloaderContext> provider)
{
super();
this.name = name;
@@ -68,6 +72,7 @@ public class PageClassloaderContext
this.classNames.addAll(classNames);
this.plasticManager = plasticProxyFactory.getPlasticManager();
this.proxyFactory = plasticProxyFactory;
+ this.provider = provider;
children = new HashSet<>();
if (plasticProxyFactory.getClassLoader() instanceof PlasticClassLoader)
{
@@ -75,6 +80,7 @@ public class PageClassloaderContext
plasticClassLoader.setTag(name);
plasticClassLoader.setFilter(this::filter);
plasticClassLoader.setAlternativeClassloading(this::alternativeClassLoading);
+ // getPlasticManager().getPool().setName(name);
if (parent != null)
{
getPlasticManager().getPool().setParent(parent.getPlasticManager().getPool());
@@ -109,6 +115,10 @@ public class PageClassloaderContext
throw new RuntimeException(e);
}
}
+ else if
(root.getPlasticManager().shouldInterceptClassLoading(className))
+ {
+ context = provider.apply(className);
+ }
return clasz;
}
diff --git
a/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManager.java
b/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManager.java
index b6680fbc5..81b3b6167 100644
---
a/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManager.java
+++
b/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManager.java
@@ -83,5 +83,11 @@ public interface PageClassloaderContextManager
* @return a Class instance.
*/
Class<?> getClassInstance(Class<?> clasz, String pageName);
+
+ /**
+ * Invalidates the <code>unknown</code> context.
+ * TODO: remove this
+ */
+ void invalidateUnknownContext();
}
diff --git
a/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManagerImpl.java
b/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManagerImpl.java
index d7753876a..ba8c3e9f6 100644
---
a/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManagerImpl.java
+++
b/tapestry-core/src/main/java/org/apache/tapestry5/services/pageload/PageClassloaderContextManagerImpl.java
@@ -78,6 +78,21 @@ public class PageClassloaderContextManagerImpl implements
PageClassloaderContext
NESTED_MERGE_COUNT.set(0);
}
+ @Override
+ public void invalidateUnknownContext()
+ {
+ for (PageClassloaderContext context : root.getChildren())
+ {
+ if (context.isUnknown())
+ {
+ componentDependencyRegistry.disableInvalidations();
+ invalidateAndFireInvalidationEvents(context);
+ componentDependencyRegistry.enableInvalidations();
+ break;
+ }
+ }
+ }
+
@Override
public void initialize(
final PageClassloaderContext root,
@@ -148,7 +163,8 @@ public class PageClassloaderContextManagerImpl implements
PageClassloaderContext
{
unknownContext = new
PageClassloaderContext(PageClassloaderContext.UNKOWN_CONTEXT_NAME, root,
Collections.emptySet(),
- plasticProxyFactoryProvider.apply(root.getClassLoader()));
+ plasticProxyFactoryProvider.apply(root.getClassLoader()),
+ this::get);
root.addChild(unknownContext);
LOGGER.debug("Unknown context: {}", unknownContext);
}
@@ -263,7 +279,8 @@ public class PageClassloaderContextManagerImpl implements
PageClassloaderContext
getContextName(className),
root,
Collections.singleton(className),
-
plasticProxyFactoryProvider.apply(root.getClassLoader()));
+
plasticProxyFactoryProvider.apply(root.getClassLoader()),
+ this::get);
}
else
{
@@ -280,16 +297,16 @@ public class PageClassloaderContextManagerImpl implements
PageClassloaderContext
getContextName(className),
parentContext,
Collections.singleton(className),
-
plasticProxyFactoryProvider.apply(parentContext.getClassLoader()));
+
plasticProxyFactoryProvider.apply(parentContext.getClassLoader()),
+ this::get);
}
context.getParent().addChild(context);
// Ensure non-page class is initialized in the correct
context and classloader.
// Pages get their own context and classloader, so this
initialization
- // is both non-needed and a cause for an NPE if it
happened.
- if (!componentClassResolver.isPage(className)/*
- && circularDependencies.isEmpty()*/)
+ // is both non-needed and a cause for an NPE if it happens.
+ if (!componentClassResolver.isPage(className))
{
loadClass(className, context);
}
@@ -448,7 +465,8 @@ public class PageClassloaderContextManagerImpl implements
PageClassloaderContext
"merged " + MERGED_COUNTER.getAndIncrement(),
parent,
classNames,
- plasticProxyFactoryProvider.apply(parent.getClassLoader()));
+ plasticProxyFactoryProvider.apply(parent.getClassLoader()),
+ this::get);
parent.addChild(merged);
