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 ca9e4c75d6440ff798b60a8cdf3f4804482767d5 Author: Thiago H. de Paula Figueiredo <[email protected]> AuthorDate: Thu Apr 27 23:37:23 2023 -0300 TAP-2742: live class reloading works, production mode doesn'r --- 583_RELEASE_NOTES.md | 1 + .../services/PropertyConduitSourceImpl.java | 1 - .../internal/plastic/PlasticClassLoader.java | 83 ++++++++- .../internal/plastic/PlasticClassPool.java | 63 +++++-- .../apache/tapestry5/plastic/PlasticManager.java | 9 + .../org/apache/tapestry5/plastic/PlasticUtils.java | 10 + .../internal/plastic/PlasticUtilsTests.groovy | 17 ++ .../tapestry5/corelib/pages/PageCatalog.java | 191 +------------------ .../tapestry5/internal/bindings/PropBinding.java | 9 - .../internal/bindings/PropBindingFactory.java | 4 +- .../internal/renderers/RequestRenderer.java | 2 + .../services/ComponentDependencyRegistryImpl.java | 96 +++++++++- .../services/ComponentInstantiatorSource.java | 8 - .../services/ComponentInstantiatorSourceImpl.java | 78 +++++--- .../internal/services/PageSourceImpl.java | 57 ++++-- .../internal/services/RequestErrorFilter.java | 77 ++++---- .../internal/services/RequestPageCacheImpl.java | 103 +++-------- .../internal/structure/ComponentPageElement.java | 1 + .../internal/transform/ComponentWorker.java | 7 +- .../tapestry5/internal/transform/ImportWorker.java | 27 +-- .../internal/transform/InjectComponentWorker.java | 16 +- .../internal/transform/InjectPageWorker.java | 13 +- .../apache/tapestry5/modules/TapestryModule.java | 18 +- .../services/pageload/PageClassloaderContext.java | 81 +++++++- .../pageload/PageClassloaderContextManager.java | 11 +- .../PageClassloaderContextManagerImpl.java | 204 ++++++++++++++++----- .../integration/app1/services/AppModule.java | 8 +- .../ComponentDependencyRegistryImplTest.java | 97 +++++++++- tapestry-core/src/test/resources/log4j.properties | 4 +- 29 files changed, 798 insertions(+), 498 deletions(-) diff --git a/583_RELEASE_NOTES.md b/583_RELEASE_NOTES.md index 7db54d7a8..882312c15 100644 --- a/583_RELEASE_NOTES.md +++ b/583_RELEASE_NOTES.md @@ -7,6 +7,7 @@ Scratch pad for changes destined for the 5.8.3 release notes page. * getValues() to MultiKey * New getLogicalName() method in ComponentClassResolver. * New getPlasticManager() method in PlasticProxyFactory +* New getPageNames() method in BeanBlockOverrideSource # Non-backward-compatible changes (but that probably won't cause problems) diff --git a/beanmodel/src/main/java/org/apache/tapestry5/beanmodel/internal/services/PropertyConduitSourceImpl.java b/beanmodel/src/main/java/org/apache/tapestry5/beanmodel/internal/services/PropertyConduitSourceImpl.java index 4b474491d..66f106660 100644 --- a/beanmodel/src/main/java/org/apache/tapestry5/beanmodel/internal/services/PropertyConduitSourceImpl.java +++ b/beanmodel/src/main/java/org/apache/tapestry5/beanmodel/internal/services/PropertyConduitSourceImpl.java @@ -1390,7 +1390,6 @@ public class PropertyConduitSourceImpl implements PropertyConduitSource @PostInjection public void listenForInvalidations(@ComponentClasses InvalidationEventHub hub) { - System.out.println("WWWWWW hub " + hub); hub.addInvalidationCallback(this::listen); } diff --git a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassLoader.java b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassLoader.java index a4ba43cc1..fdd7e9599 100644 --- a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassLoader.java +++ b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassLoader.java @@ -14,6 +14,11 @@ package org.apache.tapestry5.internal.plastic; +import java.util.function.Function; +import java.util.function.Predicate; + +import org.apache.tapestry5.plastic.PlasticUtils; + public class PlasticClassLoader extends ClassLoader { static @@ -23,11 +28,16 @@ public class PlasticClassLoader extends ClassLoader } private final ClassLoaderDelegate delegate; - - public PlasticClassLoader(ClassLoader parent, ClassLoaderDelegate delegate) + + private Predicate<String> filter; + + private Function<String, Class<?>> alternativeClassloading; + + private String tag; + + public PlasticClassLoader(ClassLoader parent, ClassLoaderDelegate delegate) { super(parent); - this.delegate = delegate; } @@ -41,9 +51,27 @@ public class PlasticClassLoader extends ClassLoader if (loadedClass != null) return loadedClass; - if (delegate.shouldInterceptClassLoading(name)) + if (shouldInterceptClassLoading(name)) { - Class<?> c = delegate.loadAndTransformClass(name); + Class<?> c = null; + if (filter == null || filter.test(name)) + { + c = delegate.loadAndTransformClass(name); + } + else if (alternativeClassloading != null) + { + c = alternativeClassloading.apply(name); + } + + if (c == null) + { + return super.loadClass(name, resolve); + } + + if (name.endsWith(".BeanEditor$Prepare")) + { + System.out.printf("LLLL Loading %s in classloader '%s' : %s\n", name, tag, this); + } if (resolve) resolveClass(c); @@ -56,6 +84,11 @@ public class PlasticClassLoader extends ClassLoader } } + private boolean shouldInterceptClassLoading(String name) { + return delegate.shouldInterceptClassLoading( + PlasticUtils.getEnclosingClassName(name)); + } + public synchronized Class<?> defineClassWithBytecode(String className, byte[] bytecode) { synchronized(getClassLoadingLock(className)) @@ -63,4 +96,44 @@ public class PlasticClassLoader extends ClassLoader return defineClass(className, bytecode, 0, bytecode.length); } } + + /** + * When alternatingClassloader is set, this classloader delegates to it the + * call to {@linkplain ClassLoader#loadClass(String)}. If it returns a non-null object, + * it's returned by <code>loadClass(String)</code>. Otherwise, it returns + * <code>super.loadClass(name)</code>. + * @since 5.8.3 + */ + public void setAlternativeClassloading(Function<String, Class<?>> alternateClassloading) + { + this.alternativeClassloading = alternateClassloading; + } + + /** + * @since 5.8.3 + */ + public void setTag(String tag) + { + this.tag = tag; + } + + /** + * When a filter is set, only classes accepted by it will be loaded by this classloader. + * Instead, it will be delegated to alternate classloading first and the parent classloader + * in case the alternate doesn't handle it. + * @since 5.8.3 + */ + public void setFilter(Predicate<String> filter) + { + this.filter = filter; + } + + @Override + public String toString() + { + final String superToString = super.toString(); + final String id = superToString.substring(superToString.indexOf('@')).trim(); + return String.format("PlasticClassLoader[%s, tag=%s, parent=%s]", id, tag, getParent()); + } + } 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 8ee547f23..1a099843c 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 @@ -14,22 +14,44 @@ package org.apache.tapestry5.internal.plastic; -import org.apache.tapestry5.internal.plastic.asm.ClassReader; -import org.apache.tapestry5.internal.plastic.asm.ClassWriter; -import org.apache.tapestry5.internal.plastic.asm.Opcodes; -import org.apache.tapestry5.internal.plastic.asm.tree.*; -import org.apache.tapestry5.plastic.*; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import java.io.BufferedInputStream; import java.io.IOException; import java.io.InputStream; import java.lang.annotation.Annotation; import java.lang.reflect.Modifier; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.ListIterator; +import java.util.Map; +import java.util.Set; +import java.util.Stack; import java.util.concurrent.CopyOnWriteArrayList; +import org.apache.tapestry5.internal.plastic.asm.ClassReader; +import org.apache.tapestry5.internal.plastic.asm.ClassWriter; +import org.apache.tapestry5.internal.plastic.asm.Opcodes; +import org.apache.tapestry5.internal.plastic.asm.tree.AbstractInsnNode; +import org.apache.tapestry5.internal.plastic.asm.tree.AnnotationNode; +import org.apache.tapestry5.internal.plastic.asm.tree.ClassNode; +import org.apache.tapestry5.internal.plastic.asm.tree.FieldInsnNode; +import org.apache.tapestry5.internal.plastic.asm.tree.InsnList; +import org.apache.tapestry5.internal.plastic.asm.tree.MethodInsnNode; +import org.apache.tapestry5.internal.plastic.asm.tree.MethodNode; +import org.apache.tapestry5.plastic.AnnotationAccess; +import org.apache.tapestry5.plastic.ClassInstantiator; +import org.apache.tapestry5.plastic.ClassType; +import org.apache.tapestry5.plastic.PlasticClassEvent; +import org.apache.tapestry5.plastic.PlasticClassListener; +import org.apache.tapestry5.plastic.PlasticClassListenerHub; +import org.apache.tapestry5.plastic.PlasticClassTransformation; +import org.apache.tapestry5.plastic.PlasticConstants; +import org.apache.tapestry5.plastic.PlasticManagerDelegate; +import org.apache.tapestry5.plastic.TransformationOption; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** * Responsible for managing a class loader that allows ASM {@link ClassNode}s * to be instantiated as runtime classes. @@ -63,6 +85,8 @@ public class PlasticClassPool implements ClassLoaderDelegate, Opcodes, PlasticCl private final StaticContext emptyStaticContext = new StaticContext(); private final List<PlasticClassListener> listeners = new CopyOnWriteArrayList<PlasticClassListener>(); + + private PlasticClassPool parent; private final Cache<String, TypeCategory> typeName2Category = new Cache<String, TypeCategory>() { @@ -482,9 +506,17 @@ public class PlasticClassPool implements ClassLoaderDelegate, Opcodes, PlasticCl if (shouldInterceptClassLoading(baseClassName)) { loader.loadClass(baseClassName); + + PlasticClassPool current = this; - BaseClassDef def = baseClassDefs.get(baseClassName); - + BaseClassDef def = current.baseClassDefs.get(baseClassName); + + while (def == null && current.parent != null) + { + current = current.parent; + def = current.baseClassDefs.get(baseClassName); + } + assert def != null; return new PlasticClassImpl(classNode, implementationClassNode, this, def.inheritanceData, def.staticContext, proxy); @@ -669,6 +701,15 @@ public class PlasticClassPool implements ClassLoaderDelegate, Opcodes, PlasticCl listeners.remove(listener); } + + /** + * Sets the parent of this instance. Only used to look up baseClassDefs. + * @since 5.8.3 + */ + public void setParent(PlasticClassPool parent) + { + this.parent = parent; + } boolean isEnabled(TransformationOption option) { diff --git a/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticManager.java b/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticManager.java index 1d42bd6b7..49249db25 100644 --- a/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticManager.java +++ b/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticManager.java @@ -356,4 +356,13 @@ public class PlasticManager implements PlasticClassListenerHub return pool.shouldInterceptClassLoading(className); } + /** + * Returns the Plastic class pool used by this instance. + * @since 5.8.3 + */ + public PlasticClassPool getPool() + { + return pool; + } + } diff --git a/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticUtils.java b/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticUtils.java index 6ed424e30..2fab09efd 100644 --- a/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticUtils.java +++ b/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticUtils.java @@ -141,4 +141,14 @@ public class PlasticUtils { return PrimitiveType.getByName(typeName) != null; } + + /** + * If the given class is an inner class, returns the enclosing class. + * Otherwise, returns the class name unchanged. + */ + public static String getEnclosingClassName(String className) + { + int index = className.indexOf('$'); + return index <= 0 ? className : className.substring(0, index); + } } diff --git a/plastic/src/test/groovy/org/apache/tapestry5/internal/plastic/PlasticUtilsTests.groovy b/plastic/src/test/groovy/org/apache/tapestry5/internal/plastic/PlasticUtilsTests.groovy index 8a2f4ea51..801f74cf2 100644 --- a/plastic/src/test/groovy/org/apache/tapestry5/internal/plastic/PlasticUtilsTests.groovy +++ b/plastic/src/test/groovy/org/apache/tapestry5/internal/plastic/PlasticUtilsTests.groovy @@ -155,4 +155,21 @@ class PlasticUtilsTests extends Specification "int" | true "java.lang.Integer" | false } + + def "getEnclosingClass #name should be #expected"() + { + + expect: + + PlasticUtils.getEnclosingClassName(name) == expected + + where: + + name | expected + + "org.apache.tapestry5.corelib.components.PropertyEditor" | "org.apache.tapestry5.corelib.components.PropertyEditor" + "org.apache.tapestry5.corelib.components.PropertyEditor\$CleanupEnvironment" | "org.apache.tapestry5.corelib.components.PropertyEditor" + + } + } diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/corelib/pages/PageCatalog.java b/tapestry-core/src/main/java/org/apache/tapestry5/corelib/pages/PageCatalog.java index e887c0da1..abaedb62d 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/corelib/pages/PageCatalog.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/corelib/pages/PageCatalog.java @@ -14,53 +14,31 @@ package org.apache.tapestry5.corelib.pages; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Set; - -import org.apache.tapestry5.MarkupWriter; import org.apache.tapestry5.alerts.AlertManager; -import org.apache.tapestry5.annotations.InjectComponent; -import org.apache.tapestry5.annotations.Persist; -import org.apache.tapestry5.annotations.Property; -import org.apache.tapestry5.annotations.UnknownActivationContextCheck; -import org.apache.tapestry5.annotations.WhitelistAccessOnly; +import org.apache.tapestry5.annotations.*; import org.apache.tapestry5.beaneditor.Validate; import org.apache.tapestry5.beanmodel.BeanModel; import org.apache.tapestry5.beanmodel.services.BeanModelSource; import org.apache.tapestry5.commons.Messages; -import org.apache.tapestry5.commons.services.InvalidationEventHub; import org.apache.tapestry5.commons.util.CollectionFactory; import org.apache.tapestry5.corelib.components.Zone; -import org.apache.tapestry5.dom.Element; -import org.apache.tapestry5.func.F; -import org.apache.tapestry5.func.Flow; -import org.apache.tapestry5.func.Mapper; -import org.apache.tapestry5.func.Predicate; -import org.apache.tapestry5.func.Reducer; +import org.apache.tapestry5.func.*; import org.apache.tapestry5.http.TapestryHttpSymbolConstants; -import org.apache.tapestry5.http.services.Request; import org.apache.tapestry5.internal.PageCatalogTotals; -import org.apache.tapestry5.internal.services.ComponentDependencyGraphvizGenerator; -import org.apache.tapestry5.internal.services.ComponentDependencyRegistry; import org.apache.tapestry5.internal.services.PageSource; import org.apache.tapestry5.internal.services.ReloadHelper; -import org.apache.tapestry5.internal.structure.ComponentPageElement; import org.apache.tapestry5.internal.structure.Page; import org.apache.tapestry5.ioc.OperationTracker; -import org.apache.tapestry5.ioc.annotations.ComponentClasses; import org.apache.tapestry5.ioc.annotations.Inject; import org.apache.tapestry5.ioc.annotations.Symbol; import org.apache.tapestry5.ioc.internal.util.InternalUtils; -import org.apache.tapestry5.runtime.Component; import org.apache.tapestry5.services.ComponentClassResolver; -import org.apache.tapestry5.services.ajax.AjaxResponseRenderer; -import org.apache.tapestry5.services.javascript.JavaScriptSupport; import org.apache.tapestry5.services.pageload.ComponentResourceSelector; +import java.util.Collection; +import java.util.List; +import java.util.Set; + /** * Lists out the currently loaded pages, using a {@link org.apache.tapestry5.corelib.components.Grid}. * Provides an option to force all pages to be loaded. In development mode, includes an option to clear the page cache. @@ -86,9 +64,6 @@ public class PageCatalog @Inject private ComponentClassResolver resolver; - - @Inject - private ComponentDependencyRegistry componentDependencyRegistry; @Inject private AlertManager alertManager; @@ -96,17 +71,8 @@ public class PageCatalog @Property private Page page; - @Property - private Page selectedPage; - - @Property - private String dependency; - @InjectComponent private Zone pagesZone; - - @InjectComponent - private Zone pageStructureZone; @Persist private Set<String> failures; @@ -124,32 +90,13 @@ public class PageCatalog @Inject private BeanModelSource beanModelSource; - + @Inject private Messages messages; @Property public static BeanModel<Page> model; - @Inject - private Request request; - - @Inject - @ComponentClasses - private InvalidationEventHub classesInvalidationEventHub; - - @Inject - private JavaScriptSupport javaScriptSupport; - - @Inject - private ComponentDependencyGraphvizGenerator componentDependencyGraphvizGenerator; - - @Inject - private ComponentClassResolver componentClassResolver; - - @Inject - private AjaxResponseRenderer ajaxResponseRenderer; - void pageLoaded() { model = beanModelSource.createDisplayModel(Page.class, messages); @@ -158,7 +105,6 @@ public class PageCatalog model.addExpression("assemblyTime", "stats.assemblyTime"); model.addExpression("componentCount", "stats.componentCount"); model.addExpression("weight", "stats.weight"); - model.add("clear", null); model.reorder("name", "selector", "assemblyTime", "componentCount", "weight"); } @@ -207,16 +153,7 @@ public class PageCatalog { return pageSource.getAllPages(); } - - Object onClearPage(String className) - { - final String logicalName = resolver.getLogicalName(className); - classesInvalidationEventHub.fireInvalidationEvent(Arrays.asList(className)); - alertManager.warn(String.format("Page %s (%s) has been cleared from the page cache", - className, logicalName)); - return pagesZone.getBody(); - } - + Object onSuccessFromSinglePageLoad() { boolean found = !F.flow(getPages()).filter(new Predicate<Page>() @@ -333,19 +270,6 @@ public class PageCatalog return pagesZone.getBody(); } - - Object onActionFromStoreDependencyInformation() - { - - componentDependencyRegistry.writeFile(); - - alertManager.warn(String.format( - "Component dependency information written to %s.", - ComponentDependencyRegistry.FILENAME)); - - return pagesZone.getBody(); - - } Object onActionFromRunGC() { @@ -367,103 +291,4 @@ public class PageCatalog { return String.format("%,.3f ms", millis); } - - public List<String> getDependencies() - { - List<String> dependencies = new ArrayList<>(componentDependencyRegistry.getDependencies(getSelectedPageClassName())); - Collections.sort(dependencies); - return dependencies; - } - - public void onPageStructure(String pageName) - { - selectedPage = pageSource.getPage(pageName); - ajaxResponseRenderer.addRender("pageStructureZone", pageStructureZone.getBody()); - } - - public String getDisplayLogicalName() - { - return getDisplayLogicalName(dependency); - } - - public String getPageClassName() - { - return getClassName(page); - } - - public String getSelectedPageClassName() - { - return getClassName(selectedPage); - } - - private String getClassName(Page page) - { - return page.getRootComponent().getComponentResources().getComponentModel().getComponentClassName(); - } - - private String getClassName(Component component) - { - return component.getComponentResources().getComponentModel().getComponentClassName(); - } - - public void onComponentTree(MarkupWriter writer) - { - render(selectedPage.getRootElement(), writer); - } - - private void render(ComponentPageElement componentPageElement, MarkupWriter writer) - { - final Element li = writer.element("li"); - final String className = getClassName(componentPageElement.getComponent()); - final Set<String> embeddedElementIds = componentPageElement.getEmbeddedElementIds(); - - if (componentPageElement.getComponent().getComponentResources().getComponentModel().isPage()) - { - li.text(componentPageElement.getPageName()); - } - else { - li.text(String.format("%s (%s)", getDisplayLogicalName(className), componentPageElement.getId())); - } - - if (!embeddedElementIds.isEmpty()) - { - writer.element("ul"); - for (String id : embeddedElementIds) - { - render(componentPageElement.getEmbeddedElement(id), writer); - } - writer.end(); - } - - writer.end(); - } - - private String getDisplayLogicalName(final String className) - { - final String logicalName = resolver.getLogicalName(className); - String displayName = logicalName; - if (logicalName == null || logicalName.trim().length() == 0) - { - if (className.contains(".base.")) - { - displayName = "(base class)"; - } - if (className.contains(".mixins.")) - { - displayName = "(mixin)"; - } - } - return displayName; - } - - public String getLogicalName(String className) - { - return resolver.getLogicalName(className); - } - - public String getGraphvizValue() - { - return componentDependencyGraphvizGenerator.generate(getClassName(selectedPage)); - } - } diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBinding.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBinding.java index 973c42b41..8a7778153 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBinding.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBinding.java @@ -16,7 +16,6 @@ package org.apache.tapestry5.internal.bindings; import java.lang.annotation.Annotation; import java.lang.reflect.Type; -import java.util.concurrent.atomic.AtomicInteger; import org.apache.tapestry5.beanmodel.PropertyConduit; import org.apache.tapestry5.beanmodel.PropertyConduit2; @@ -40,10 +39,6 @@ public class PropBinding extends AbstractBinding implements InternalPropBinding private boolean invariant; private final String expression; - - // TODO remove - private static final AtomicInteger COUNTER = new AtomicInteger(); - private final int id; public PropBinding(final Location location, final Object root, final PropertyConduit conduit, final String expression, final String toString) { @@ -55,10 +50,6 @@ public class PropBinding extends AbstractBinding implements InternalPropBinding this.toString = toString; invariant = conduit.getAnnotation(Invariant.class) != null; - - System.out.println("TTTTT new PropBinding: " + root.getClass().getSimpleName() + ":" + expression); - id = COUNTER.getAndIncrement(); - } /** diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBindingFactory.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBindingFactory.java index c8e51ffcb..e2bcd29e7 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBindingFactory.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/bindings/PropBindingFactory.java @@ -68,10 +68,10 @@ public class PropBindingFactory implements BindingFactory try { final PageClassloaderContext context = pageClassloaderContextManager.get(className); - System.out.printf("XXXXX Target class (before): %s classloader : %s\n", targetClass.getSimpleName(), targetClass.getClassLoader()); +// System.out.printf("XXXXX Target class (before): %s classloader : %s\n", targetClass.getSimpleName(), targetClass.getClassLoader()); targetClass = context.getProxyFactory() .getClassLoader().loadClass(className); - System.out.printf("XXXXX Target class (after) : %s classloader : %s context %s\n", targetClass.getSimpleName(), targetClass.getClassLoader(), context.getName()); +// System.out.printf("XXXXX Target class (after) : %s classloader : %s context %s\n", targetClass.getSimpleName(), targetClass.getClassLoader(), context.getName()); } catch (ClassNotFoundException e) { throw new TapestryException(e.getMessage(), e); diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/renderers/RequestRenderer.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/renderers/RequestRenderer.java index 326ca7226..c0b0e956f 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/renderers/RequestRenderer.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/renderers/RequestRenderer.java @@ -77,6 +77,8 @@ public class RequestRenderer implements ObjectRenderer<Request> headers(request, writer); attributes(request, writer); context(writer); + + // TODO: remove methods below pageClassloaderContext(writer); pages(writer); } diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentDependencyRegistryImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentDependencyRegistryImpl.java index a694cdb71..9e5198cef 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentDependencyRegistryImpl.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentDependencyRegistryImpl.java @@ -25,6 +25,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Map; @@ -76,6 +77,10 @@ public class ComponentDependencyRegistryImpl implements ComponentDependencyRegis // Key is a component, values are the components that depend on it. final private Map<String, Set<String>> map; + // Dependencies that are only for invalidation (i.e. from one component to + // a page injected through @InjectPage + final private Set<InvalidationOnlyDependency> invalidationOnlyDependencies; + // Cache to check which classes were already processed or not. final private Set<String> alreadyProcessed; @@ -104,6 +109,7 @@ public class ComponentDependencyRegistryImpl implements ComponentDependencyRegis this.pageClassloaderContextManager = pageClassloaderContextManager; map = new HashMap<>(); alreadyProcessed = new HashSet<>(); + invalidationOnlyDependencies = new HashSet<InvalidationOnlyDependency>(); this.plasticManager = plasticManager; this.resolver = componentClassResolver; this.templateParser = templateParser; @@ -150,6 +156,7 @@ public class ComponentDependencyRegistryImpl implements ComponentDependencyRegis public void register(Class<?> component) { + final String className = component.getName(); final Set<Class<?>> furtherDependencies = new HashSet<>(); Consumer<Class<?>> processClass = furtherDependencies::add; Consumer<String> processClassName = s -> { @@ -168,15 +175,22 @@ public class ComponentDependencyRegistryImpl implements ComponentDependencyRegis for (Field field : component.getDeclaredFields()) { - // Component and page injection annotation - if (field.isAnnotationPresent(InjectPage.class) || - field.isAnnotationPresent(InjectComponent.class)) + // Component injection annotation + if (field.isAnnotationPresent(InjectComponent.class)) { final Class<?> dependency = field.getType(); add(component, dependency); processClass.accept(dependency); } + // Page injection annotation + if (field.isAnnotationPresent(InjectComponent.class)) + { + final Class<?> dependency = field.getType(); + invalidationOnlyDependencies.add( + new InvalidationOnlyDependency(className, dependency.getName())); + } + // @Component registerComponentInstance(field, processClassName); @@ -195,14 +209,16 @@ public class ComponentDependencyRegistryImpl implements ComponentDependencyRegis add(component, superclass); } - alreadyProcessed.add(component.getName()); + Set<String> dynamicDependencies; + + alreadyProcessed.add(className); for (Class<?> dependency : furtherDependencies) { // Avoid infinite recursion final String dependencyClassName = dependency.getName(); if (!alreadyProcessed.contains(dependencyClassName) - && !plasticManager.shouldInterceptClassLoading(dependency.getName())) + && plasticManager.shouldInterceptClassLoading(dependency.getName())) { register(dependency); } @@ -443,6 +459,18 @@ public class ComponentDependencyRegistryImpl implements ComponentDependencyRegis } } } + synchronized (invalidationOnlyDependencies) + { + final Iterator<InvalidationOnlyDependency> iterator = invalidationOnlyDependencies.iterator(); + while (iterator.hasNext()) + { + InvalidationOnlyDependency dependency = iterator.next(); + if (dependency.className.equals(className) || dependency.dependency.equals(className)) + { + iterator.remove(); + } + } + } } @Override @@ -467,10 +495,16 @@ public class ComponentDependencyRegistryImpl implements ComponentDependencyRegis @Override public Set<String> getDependencies(String className) { - return map.entrySet().stream() + Set<String> dependencies = Collections.emptySet(); + if (alreadyProcessed.contains(className)) + { + dependencies = map.entrySet().stream() .filter(e -> e.getValue().contains(className)) .map(e -> e.getKey()) .collect(Collectors.toSet()); + } + + return dependencies; } private void add(ComponentPageElement component, ComponentPageElement dependency) @@ -491,7 +525,7 @@ public class ComponentDependencyRegistryImpl implements ComponentDependencyRegis } } - private void add(Class component, Class dependency) + private void add(Class<?> component, Class<?> dependency) { if (plasticManager.shouldInterceptClassLoading(dependency.getName())) { @@ -536,11 +570,13 @@ public class ComponentDependencyRegistryImpl implements ComponentDependencyRegis } // Don't invalidate component dependency information when // PageClassloaderContextManager is merging contexts + // TODO is this still needed since the inception of INVALIDATIONS_ENABLED? else if (!pageClassloaderContextManager.isMerging()) { furtherDependents = new ArrayList<>(); for (String resource : resources) { + final Set<String> dependents = getDependents(resource); for (String furtherDependent : dependents) { @@ -549,6 +585,15 @@ public class ComponentDependencyRegistryImpl implements ComponentDependencyRegis furtherDependents.add(furtherDependent); } } + + for (InvalidationOnlyDependency invalidationOnlyDependency : invalidationOnlyDependencies) + { + if (invalidationOnlyDependency.dependency.equals(resource)) + { + furtherDependents.add(invalidationOnlyDependency.dependency); + } + } + clear(resource); } } @@ -783,5 +828,42 @@ public class ComponentDependencyRegistryImpl implements ComponentDependencyRegis } + final private class InvalidationOnlyDependency + { + + private String className; + private String dependency; + + public InvalidationOnlyDependency(String className, String dependency) + { + super(); + this.className = className; + this.dependency = dependency; + } + + @Override + public int hashCode() + { + final int prime = 31; + int result = 1; + result = prime * result + Objects.hash(className, dependency); + return result; + } + + @Override + public boolean equals(Object obj) + { + if (this == obj) + { + return true; + } + if (!(obj instanceof InvalidationOnlyDependency)) { + return false; + } + InvalidationOnlyDependency other = (InvalidationOnlyDependency) obj; + return Objects.equals(className, other.className) && Objects.equals(dependency, other.dependency); + } + + } } diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSource.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSource.java index 1291d7444..ed7a40f34 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSource.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentInstantiatorSource.java @@ -12,8 +12,6 @@ package org.apache.tapestry5.internal.services; -import java.util.List; - import org.apache.tapestry5.commons.services.PlasticProxyFactory; import org.apache.tapestry5.ioc.annotations.UsesMappedConfiguration; import org.apache.tapestry5.services.transform.ControlledPackageType; @@ -70,10 +68,4 @@ public interface ComponentInstantiatorSource * @since 5.3 */ void forceComponentInvalidation(); - - /** - * Invalidates the caches for a given list of class names. - * @since 5.8.3 - */ - void invalidate(List<String> classNames); } 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 588c99dde..63edfcb0b 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 @@ -117,6 +117,10 @@ public final class ComponentInstantiatorSourceImpl implements ComponentInstantia private PageClassloaderContext rootPageClassloaderContext; private PlasticProxyFactoryProxy plasticProxyFactoryProxy; + + private ComponentDependencyRegistry componentDependencyRegistry; + + private static final ThreadLocal<String> CURRENT_PAGE = ThreadLocal.withInitial(() -> null); /** * Map from class name to Instantiator. @@ -161,7 +165,9 @@ public final class ComponentInstantiatorSourceImpl implements ComponentInstantia InternalComponentInvalidationEventHub invalidationHub, - PageClassloaderContextManager pageClassloaderContextManager + PageClassloaderContextManager pageClassloaderContextManager, + + ComponentDependencyRegistry componentDependencyRegistry ) { this.parent = proxyFactory.getClassLoader(); @@ -174,6 +180,7 @@ public final class ComponentInstantiatorSourceImpl implements ComponentInstantia this.productionMode = productionMode; this.resolver = resolver; this.pageClassloaderContextManager = pageClassloaderContextManager; + this.componentDependencyRegistry = componentDependencyRegistry; // For now, we just need the keys of the configuration. When there are more types of controlled // packages, we'll need to do more. @@ -191,7 +198,7 @@ public final class ComponentInstantiatorSourceImpl implements ComponentInstantia @PostInjection public void listenForUpdates(UpdateListenerHub hub) { -// invalidationHub.addInvalidationCallback(this); + invalidationHub.addInvalidationCallback(this::invalidate); hub.addUpdateListener(this); } @@ -224,27 +231,32 @@ public final class ComponentInstantiatorSourceImpl implements ComponentInstantia } } - @Override - public void invalidate(final List<String> classNames) { + private List<String> invalidate(final List<String> classNames) { + + final String currentPage = CURRENT_PAGE.get(); final Iterator<Entry<String, Instantiator>> classToInstantiatorIterator = classToInstantiator.entrySet().iterator(); while (classToInstantiatorIterator.hasNext()) { - if (classNames.contains(classToInstantiatorIterator.next().getKey())) + final String className = classToInstantiatorIterator.next().getKey(); + if (!className.equals(currentPage) && classNames.contains(className)) { +// System.out.println("WWWWW Removing class instantiator " + className); classToInstantiatorIterator.remove(); } } - // TODO: fix this -// final Iterator<Entry<String, ComponentModel>> classToModelIterator = classToModel.entrySet().iterator(); -// while (classToModelIterator.hasNext()) -// { -// if (classNames.contains(classToModelIterator.next().getKey())) -// { -// classToModelIterator.remove(); -// } -// } + final Iterator<Entry<String, ComponentModel>> classToModelIterator = classToModel.entrySet().iterator(); + while (classToModelIterator.hasNext()) + { + final String className = classToModelIterator.next().getKey(); + if (!className.equals(currentPage) && classNames.contains(className)) + { +// System.out.println("WWWWW Removing class model " + className); + classToModelIterator.remove(); + } + } + return Collections.emptyList(); } public void forceComponentInvalidation() @@ -252,14 +264,14 @@ public final class ComponentInstantiatorSourceImpl implements ComponentInstantia changeTracker.clear(); invalidationHub.classInControlledPackageHasChanged(); pageClassloaderContextManager.clear(); + classToModel.clear(); } public void run() { changeTracker.clear(); classToInstantiator.clear(); - // TODO fix this -// classToModel.clear(); + classToModel.clear(); pageClassloaderContextManager.clear(); initializeService(); } @@ -312,6 +324,9 @@ public final class ComponentInstantiatorSourceImpl implements ComponentInstantia { return classToInstantiator.computeIfAbsent(className, this::createInstantiatorForClass); } + + private static final ThreadLocal<Set<String>> OPEN_INSTANTIATORS = + ThreadLocal.withInitial(HashSet::new); private Instantiator createInstantiatorForClass(final String className) { @@ -320,14 +335,33 @@ public final class ComponentInstantiatorSourceImpl implements ComponentInstantia { public Instantiator invoke() { + // Force the creation of the class (and the transformation of the class). This will first // trigger transformations of any base classes. - - final PageClassloaderContext context = pageClassloaderContextManager.get(className); - final ClassInstantiator<Component> plasticInstantiator = - context.getPlasticManager().getClassInstantiator(className); - + + OPEN_INSTANTIATORS.get().add(className); + + componentDependencyRegistry.disableInvalidations(); + PageClassloaderContext context = pageClassloaderContextManager.get(className); + componentDependencyRegistry.enableInvalidations(); + + // Make sure the dependencies have been processed in case + // there was some invalidation going on and they're not there. + + final Set<String> dependencies = componentDependencyRegistry.getDependencies(className); + for (String dependency : dependencies) + { + if (!OPEN_INSTANTIATORS.get().contains(dependency)) + { +// System.out.println("TTTTT calling createInstaniatoForClass " + dependency); + createInstantiatorForClass(dependency); + } + } + + ClassInstantiator<Component> plasticInstantiator = context.getPlasticManager().getClassInstantiator(className); final ComponentModel model = classToModel.get(className); + + OPEN_INSTANTIATORS.get().remove(className); return new Instantiator() { @@ -345,7 +379,7 @@ public final class ComponentInstantiatorSourceImpl implements ComponentInstantia @Override public String toString() { - return String.format("[Instantiator[%s]", className); + return String.format("[Instantiator[%s:%s]", className, context); } }; } 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 79b2a51dd..b442646c6 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 @@ -139,27 +139,44 @@ public class PageSourceImpl implements PageSource // different threads. The last built one will "evict" the others from the page cache, // and the earlier ones will be GCed. -// if (!productionMode) -// { -// // Make sure you get a fresh version of the class before processing its -// // dependencies -// final String className = componentClassResolver.resolvePageNameToClassName(canonicalPageName); -// pageClassloaderContextManager.clear(className); -// final PageClassloaderContext context = pageClassloaderContextManager.get(className); -// final Class<?> clasz; -// try { -// clasz = context.getProxyFactory().getClassLoader().loadClass(className); -// pageClassloaderContextManager.invalidateAndFireInvalidationEvents(context); -// componentDependencyRegistry.register(clasz); -// pageClassloaderContextManager.get(className); -// } catch (ClassNotFoundException e) -// { -// logger.error(e.getMessage(), e); -// } -// -// } + // TODO: maybe remove this + if (!productionMode) + { + + final String className = componentClassResolver.resolvePageNameToClassName(canonicalPageName); + if (!componentDependencyRegistry.contains(canonicalPageName)) + { + // Make sure you get a fresh version of the class before processing its + // dependencies + pageClassloaderContextManager.clear(className); + final PageClassloaderContext context = pageClassloaderContextManager.get(className); + final Class<?> clasz; + try { + + clasz = context.getProxyFactory().getClassLoader().loadClass(className); + pageClassloaderContextManager.invalidateAndFireInvalidationEvents(context); + componentDependencyRegistry.register(clasz); + + // The call to register() above will potentially cause some dependencies + // to wrongly end in the the page's context's classloader instead of one of its parent + // context/classloader. That's why we need to invalidate the page's current + // context and classloader and create a new instance of each. +// componentDependencyRegistry.disableInvalidations(); +// final PageClassloaderContext toInvalidate = +// pageClassloaderContextManager.get(className); +// pageClassloaderContextManager.invalidate(toInvalidate); +// componentDependencyRegistry.disableInvalidations(); + pageClassloaderContextManager.get(className); + } catch (ClassNotFoundException e) + { + throw new RuntimeException(e); + } + + } + + } - page = pageLoader.loadPage(canonicalPageName, selector); + page = pageLoader.loadPage(canonicalPageName, selector); ref = new SoftReference<Page>(page); diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/RequestErrorFilter.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/RequestErrorFilter.java index 32fa090ef..f1fdc6c87 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/RequestErrorFilter.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/RequestErrorFilter.java @@ -65,44 +65,44 @@ public class RequestErrorFilter implements RequestFilter catch (Throwable ex) { -// if (request.getParameter(QUERY_PARAMETER) == null) -// { -// -// Throwable rootCause = ex.getCause(); -// String classToInvalidate = getClassToInvalidate(rootCause); -// -// if (classToInvalidate != null) -// { -// -// final List<String> classesToInvalidate = -// Arrays.asList(classToInvalidate, ExceptionReport.class.getName()); -// componentInstantiatorSource.invalidate(classesToInvalidate); -// classesInvalidationHub.fireInvalidationEvent(classesToInvalidate); -// -// Link link = null; -// -// final ComponentEventRequestParameters componentEventParameters = componentEventLinkEncoder.decodeComponentEventRequest(request); -// if (componentEventParameters != null) -// { -// link = componentEventLinkEncoder.createComponentEventLink(componentEventParameters, false); -// } -// -// final PageRenderRequestParameters pageRenderParameters = componentEventLinkEncoder.decodePageRenderRequest(request); -// if (pageRenderParameters != null) -// { -// link = componentEventLinkEncoder.createPageRenderLink(pageRenderParameters); -// } -// -// if (link != null) -// { -// link.addParameter(QUERY_PARAMETER, "true"); -// response.sendRedirect(link); -// return true; -// } -// -// } -// -// } + // TODO: evaluate removing this + if (request.getParameter(QUERY_PARAMETER) == null) + { + + Throwable rootCause = ex.getCause(); + String classToInvalidate = getClassToInvalidate(rootCause); + + if (classToInvalidate != null) + { + + final List<String> classesToInvalidate = + Arrays.asList(classToInvalidate, ExceptionReport.class.getName()); + classesInvalidationHub.fireInvalidationEvent(classesToInvalidate); + + Link link = null; + + final ComponentEventRequestParameters componentEventParameters = componentEventLinkEncoder.decodeComponentEventRequest(request); + if (componentEventParameters != null) + { + link = componentEventLinkEncoder.createComponentEventLink(componentEventParameters, false); + } + + final PageRenderRequestParameters pageRenderParameters = componentEventLinkEncoder.decodePageRenderRequest(request); + if (pageRenderParameters != null) + { + link = componentEventLinkEncoder.createPageRenderLink(pageRenderParameters); + } + + if (link != null) + { + link.addParameter(QUERY_PARAMETER, "true"); + response.sendRedirect(link); + return true; + } + + } + + } // Most of the time, we've got exception linked up the kazoo ... but when ClassLoaders // get involved, things go screwy. Exceptions when transforming classes can cause @@ -110,6 +110,7 @@ public class RequestErrorFilter implements RequestFilter // TAPESTRY-2078 System.out.println("YYYYYY" + pageClassloaderContextManager.getRoot().toRecursiveString()); + ex.printStackTrace(); Throwable exceptionToReport = attachNewCause(ex, internalRequestGlobals.getClassLoaderException()); diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/RequestPageCacheImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/RequestPageCacheImpl.java index a0c06d4f5..99563acb0 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/RequestPageCacheImpl.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/RequestPageCacheImpl.java @@ -14,24 +14,22 @@ package org.apache.tapestry5.internal.services; +import java.util.Collections; +import java.util.List; import java.util.Map; -import java.util.Set; -import org.apache.tapestry5.SymbolConstants; +import org.apache.tapestry5.commons.services.InvalidationEventHub; import org.apache.tapestry5.commons.util.CollectionFactory; import org.apache.tapestry5.commons.util.ExceptionUtils; -import org.apache.tapestry5.corelib.pages.ExceptionReport; import org.apache.tapestry5.http.services.RequestGlobals; import org.apache.tapestry5.internal.InternalConstants; import org.apache.tapestry5.internal.structure.Page; import org.apache.tapestry5.ioc.ScopeConstants; +import org.apache.tapestry5.ioc.annotations.ComponentClasses; import org.apache.tapestry5.ioc.annotations.PostInjection; import org.apache.tapestry5.ioc.annotations.Scope; -import org.apache.tapestry5.ioc.annotations.Symbol; import org.apache.tapestry5.ioc.services.PerthreadManager; import org.apache.tapestry5.services.ComponentClassResolver; -import org.apache.tapestry5.services.pageload.PageClassloaderContext; -import org.apache.tapestry5.services.pageload.PageClassloaderContextManager; import org.slf4j.Logger; /** @@ -55,30 +53,21 @@ public class RequestPageCacheImpl implements RequestPageCache, Runnable private final Map<String, Page> cache = CollectionFactory.newMap(); - private final PageClassloaderContextManager pageClassloaderContextManager; - - private final ComponentDependencyRegistry componentDependencyRegistry; - - private final boolean productionMode; - - public RequestPageCacheImpl(Logger logger, ComponentClassResolver resolver, PageSource pageSource, RequestGlobals requestGlobals, - PageClassloaderContextManager pageClassloaderContextManager, - ComponentDependencyRegistry componentDependencyRegistry, - @Symbol(SymbolConstants.PRODUCTION_MODE) boolean productionMode) + public RequestPageCacheImpl(Logger logger, ComponentClassResolver resolver, + PageSource pageSource, RequestGlobals requestGlobals) { this.logger = logger; this.resolver = resolver; this.pageSource = pageSource; this.requestGlobals = requestGlobals; - this.pageClassloaderContextManager = pageClassloaderContextManager; - this.productionMode = productionMode; - this.componentDependencyRegistry = componentDependencyRegistry; } @PostInjection - public void listenForThreadCleanup(PerthreadManager perthreadManager) + public void listenForThreadCleanup(PerthreadManager perthreadManager, + @ComponentClasses InvalidationEventHub classesHub) { perthreadManager.addThreadCleanupCallback(this); + classesHub.addInvalidationCallback(this::listen); } public void run() @@ -103,69 +92,13 @@ public class RequestPageCacheImpl implements RequestPageCache, Runnable if (page == null) { -// if (!productionMode) -// { -// final String className = resolver.getClassName(canonical); -// if (!componentDependencyRegistry.contains(className)) -// { -// final PageClassloaderContext context = pageClassloaderContextManager.get(className); -// -// try { -// Class<?> clasz = context.getProxyFactory().getClassLoader().loadClass(className); -// componentDependencyRegistry.register(clasz); -// if (context.isUnknown()) -// { -// pageClassloaderContextManager.invalidateAndFireInvalidationEvents(context); -// } -// componentDependencyRegistry.register(clasz); -// if (context.isUnknown()) -// { -// pageClassloaderContextManager.get(className); -// } -// } catch (ClassNotFoundException e) { -// logger.warn("Exception while loading class " + className, e); -// } -// } -// } - page = pageSource.getPage(canonical); - -// // In production mode, we don't have the unknown context nor -// // page invalidation. -// if (!productionMode) -// { -// -// // If the page is in the unknown page classloader context, it means we don't -// // know its dependencies yet. Since we just got an instance assembled, -// // let's throw it away and rebuild with known depedencies. This way, -// // we avoid ClassCastExceptions later on classes used by it. -// final String className = page.getRootComponent().getComponentResources().getComponentModel().getComponentClassName(); -// -// final PageClassloaderContext context = pageClassloaderContextManager.get(className); -// if (context.isUnknown() && false) -// { -// System.out.println("XXXXX Before unknown invalidation"); -// System.out.println(root.toRecursiveString()); -// componentDependencyRegistry.disableInvalidations(); -// pageClassloaderContextManager.invalidateAndFireInvalidationEvents(context); -// componentDependencyRegistry.enableInvalidations(); -// // Force processing the classloader context for this page so it's ready -// // when the new page instance is created -// pageClassloaderContextManager.get(className); -// page = pageSource.getPage(canonical); -// } -// -// } try { page.attached(); - } - catch (Throwable t) + } catch (Throwable t) { - System.out.println("XXXX " + pageClassloaderContextManager.getRoot()); - cache.remove(canonical); - pageClassloaderContextManager.get(ExceptionReport.class.getName()); throw new RuntimeException(String.format("Unable to attach page %s: %s", canonical, ExceptionUtils.toMessage(t)), t); } @@ -181,4 +114,20 @@ public class RequestPageCacheImpl implements RequestPageCache, Runnable return page; } + + private List<String> listen(List<String> resources) + { + // TODO: we probably don't need this anymore + for (String resource : resources) + { + if (resolver.isPage(resource)) + { + final String canonicalName = resolver.canonicalizePageName( + resolver.getLogicalName(resource)); + cache.remove(canonicalName); + } + } + return Collections.emptyList(); + } + } diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/ComponentPageElement.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/ComponentPageElement.java index 1e17fe90b..2fdf0b1ff 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/ComponentPageElement.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/ComponentPageElement.java @@ -106,6 +106,7 @@ public interface ComponentPageElement extends ComponentResourcesCommon, Internal */ ComponentPageElement getEmbeddedElement(String id); + // TODO: remove /** * Returns the ids of all embedded elements defined within the component. * @since 5.8.3 diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ComponentWorker.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ComponentWorker.java index bd4628ed9..54a4a487f 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ComponentWorker.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ComponentWorker.java @@ -25,7 +25,6 @@ import org.apache.tapestry5.commons.util.CollectionFactory; import org.apache.tapestry5.commons.util.CommonsUtils; import org.apache.tapestry5.internal.KeyValue; import org.apache.tapestry5.internal.TapestryInternalUtils; -import org.apache.tapestry5.internal.services.ComponentDependencyRegistry; import org.apache.tapestry5.ioc.Orderable; import org.apache.tapestry5.ioc.internal.util.InternalUtils; import org.apache.tapestry5.model.ComponentModel; @@ -47,13 +46,10 @@ import java.util.List; public class ComponentWorker implements ComponentClassTransformWorker2 { private final ComponentClassResolver resolver; - - private final ComponentDependencyRegistry componentDependencyRegistry; - public ComponentWorker(ComponentClassResolver resolver, ComponentDependencyRegistry componentDependencyRegistry) + public ComponentWorker(ComponentClassResolver resolver) { this.resolver = resolver; - this.componentDependencyRegistry = componentDependencyRegistry; } public void transform(PlasticClass plasticClass, TransformationSupport support, MutableComponentModel model) @@ -61,7 +57,6 @@ public class ComponentWorker implements ComponentClassTransformWorker2 for (PlasticField field : plasticClass.getFieldsWithAnnotation(Component.class)) { transformField(plasticClass, model, field); - componentDependencyRegistry.register(field, model); } } diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ImportWorker.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ImportWorker.java index c7038c2b7..a3801a6ea 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ImportWorker.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ImportWorker.java @@ -14,13 +14,11 @@ package org.apache.tapestry5.internal.transform; import org.apache.tapestry5.Asset; import org.apache.tapestry5.ComponentResources; -import org.apache.tapestry5.SymbolConstants; import org.apache.tapestry5.annotations.Import; import org.apache.tapestry5.annotations.SetupRender; import org.apache.tapestry5.func.F; import org.apache.tapestry5.func.Mapper; import org.apache.tapestry5.func.Worker; -import org.apache.tapestry5.internal.services.assets.ResourceChangeTracker; import org.apache.tapestry5.ioc.services.SymbolSource; import org.apache.tapestry5.model.MutableComponentModel; import org.apache.tapestry5.plastic.*; @@ -46,10 +44,6 @@ public class ImportWorker implements ComponentClassTransformWorker2 private final SymbolSource symbolSource; private final AssetSource assetSource; - - private final ResourceChangeTracker resourceChangeTracker; - - private final boolean productionMode; private final Worker<Asset> importLibrary = new Worker<Asset>() { @@ -75,27 +69,21 @@ public class ImportWorker implements ComponentClassTransformWorker2 } }; - public ImportWorker(JavaScriptSupport javascriptSupport, SymbolSource symbolSource, AssetSource assetSource, - ResourceChangeTracker resourceChangeTracker) + public ImportWorker(JavaScriptSupport javascriptSupport, SymbolSource symbolSource, AssetSource assetSource) { this.javascriptSupport = javascriptSupport; this.symbolSource = symbolSource; this.assetSource = assetSource; - this.resourceChangeTracker = resourceChangeTracker; - this.productionMode = Boolean.valueOf(symbolSource.valueForSymbol(SymbolConstants.PRODUCTION_MODE)); } public void transform(PlasticClass componentClass, TransformationSupport support, MutableComponentModel model) { - resourceChangeTracker.setCurrentClassName(model.getComponentClassName()); processClassAnnotationAtSetupRenderPhase(componentClass, model); for (PlasticMethod m : componentClass.getMethodsWithAnnotation(Import.class)) { decorateMethod(componentClass, model, m); } - - resourceChangeTracker.clearCurrentClassName(); } private void processClassAnnotationAtSetupRenderPhase(PlasticClass componentClass, MutableComponentModel model) @@ -124,6 +112,8 @@ public class ImportWorker implements ComponentClassTransformWorker2 { importStacks(method, annotation.stack()); + String libraryName = model.getLibraryName(); + importLibraries(componentClass, model, method, annotation.library()); importStylesheets(componentClass, model, method, annotation.stylesheet()); @@ -274,7 +264,6 @@ public class ImportWorker implements ComponentClassTransformWorker2 private void addMethodAssetOperationAdvice(PlasticMethod method, final FieldHandle access, final Worker<Asset> operation) { - final String className = method.getPlasticClass().getClassName(); method.addAdvice(new MethodAdvice() { public void advise(MethodInvocation invocation) @@ -283,17 +272,7 @@ public class ImportWorker implements ComponentClassTransformWorker2 Asset[] assets = (Asset[]) access.get(invocation.getInstance()); - if (!productionMode) - { - resourceChangeTracker.setCurrentClassName(className); - } - F.flow(assets).each(operation); - - if (!productionMode) - { - resourceChangeTracker.clearCurrentClassName(); - } } }); } diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/InjectComponentWorker.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/InjectComponentWorker.java index 2240a418b..b18518ca9 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/InjectComponentWorker.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/InjectComponentWorker.java @@ -19,14 +19,9 @@ import org.apache.tapestry5.annotations.InjectComponent; import org.apache.tapestry5.commons.util.DifferentClassVersionsException; import org.apache.tapestry5.commons.util.UnknownValueException; import org.apache.tapestry5.internal.services.ComponentClassCache; -import org.apache.tapestry5.internal.services.ComponentDependencyRegistry; import org.apache.tapestry5.ioc.internal.util.InternalUtils; import org.apache.tapestry5.model.MutableComponentModel; -import org.apache.tapestry5.plastic.ComputedValue; -import org.apache.tapestry5.plastic.FieldConduit; -import org.apache.tapestry5.plastic.InstanceContext; -import org.apache.tapestry5.plastic.PlasticClass; -import org.apache.tapestry5.plastic.PlasticField; +import org.apache.tapestry5.plastic.*; import org.apache.tapestry5.runtime.Component; import org.apache.tapestry5.services.transform.ComponentClassTransformWorker2; import org.apache.tapestry5.services.transform.TransformationSupport; @@ -124,13 +119,9 @@ public class InjectComponentWorker implements ComponentClassTransformWorker2 private final Logger logger; - private final ComponentDependencyRegistry componentDependencyRegistry; - - public InjectComponentWorker(ComponentClassCache classCache, - ComponentDependencyRegistry componentDependencyRegistry, final Logger logger) + public InjectComponentWorker(ComponentClassCache classCache, final Logger logger) { this.classCache = classCache; - this.componentDependencyRegistry = componentDependencyRegistry; this.logger = logger; } @@ -161,9 +152,6 @@ public class InjectComponentWorker implements ComponentClassTransformWorker2 }; field.setComputedConduit(provider); - - componentDependencyRegistry.register(field, model); - } } diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/InjectPageWorker.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/InjectPageWorker.java index 6b9870663..dc36b58f4 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/InjectPageWorker.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/InjectPageWorker.java @@ -16,7 +16,6 @@ package org.apache.tapestry5.internal.transform; import org.apache.tapestry5.annotations.InjectPage; import org.apache.tapestry5.commons.ObjectCreator; -import org.apache.tapestry5.internal.services.ComponentDependencyRegistry; import org.apache.tapestry5.ioc.internal.util.InternalUtils; import org.apache.tapestry5.ioc.services.PerthreadManager; import org.apache.tapestry5.model.MutableComponentModel; @@ -66,26 +65,22 @@ public class InjectPageWorker implements ComponentClassTransformWorker2 private final PerthreadManager perThreadManager; - private final ComponentDependencyRegistry componentDependencyRegistry; - - public InjectPageWorker(ComponentSource componentSource, ComponentClassResolver resolver, PerthreadManager perThreadManager, - ComponentDependencyRegistry componentDependencyRegistry) + public InjectPageWorker(ComponentSource componentSource, ComponentClassResolver resolver, PerthreadManager perThreadManager) { this.componentSource = componentSource; this.resolver = resolver; this.perThreadManager = perThreadManager; - this.componentDependencyRegistry = componentDependencyRegistry; } public void transform(PlasticClass plasticClass, TransformationSupport support, MutableComponentModel model) { for (PlasticField field : plasticClass.getFieldsWithAnnotation(InjectPage.class)) { - addInjectedPage(field, model); + addInjectedPage(field); } } - private void addInjectedPage(PlasticField field, MutableComponentModel model) + private void addInjectedPage(PlasticField field) { InjectPage annotation = field.getAnnotation(InjectPage.class); @@ -99,7 +94,5 @@ public class InjectPageWorker implements ComponentClassTransformWorker2 .resolvePageClassNameToPageName(field.getTypeName()) : pageName; field.setConduit(new InjectedPageConduit(field.getPlasticClass().getClassName(), fieldName, injectedPageName)); - - componentDependencyRegistry.register(field, model); } } 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 9c4a55827..ba60eb924 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 @@ -20,15 +20,19 @@ import java.math.BigInteger; import java.net.URL; import java.text.DateFormat; import java.text.SimpleDateFormat; +import java.util.Arrays; import java.util.Calendar; import java.util.Collection; +import java.util.Collections; import java.util.Date; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Properties; import java.util.Set; import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.apache.tapestry5.Asset; import org.apache.tapestry5.BindingConstants; @@ -104,7 +108,12 @@ import org.apache.tapestry5.commons.services.TypeCoercer; import org.apache.tapestry5.commons.util.AvailableValues; import org.apache.tapestry5.commons.util.CollectionFactory; import org.apache.tapestry5.commons.util.StrategyRegistry; +import org.apache.tapestry5.corelib.components.BeanEditor; +import org.apache.tapestry5.corelib.components.PropertyDisplay; +import org.apache.tapestry5.corelib.components.PropertyEditor; import org.apache.tapestry5.corelib.data.SecureOption; +import org.apache.tapestry5.corelib.pages.PropertyDisplayBlocks; +import org.apache.tapestry5.corelib.pages.PropertyEditBlocks; import org.apache.tapestry5.grid.GridConstants; import org.apache.tapestry5.grid.GridDataSource; import org.apache.tapestry5.http.Link; @@ -364,6 +373,7 @@ import org.apache.tapestry5.services.meta.FixedExtractor; import org.apache.tapestry5.services.meta.MetaDataExtractor; import org.apache.tapestry5.services.meta.MetaWorker; import org.apache.tapestry5.services.pageload.PageClassloaderContextManager; +import org.apache.tapestry5.services.pageload.PageClassloaderContextManagerImpl; import org.apache.tapestry5.services.pageload.PreloaderMode; import org.apache.tapestry5.services.rest.MappedEntityManager; import org.apache.tapestry5.services.rest.OpenApiDescriptionGenerator; @@ -687,7 +697,7 @@ public final class TapestryModule configuration.addInstance("ApplicationState", ApplicationStateWorker.class); configuration.addInstance("Environment", EnvironmentalWorker.class); - configuration.add("Component", new ComponentWorker(resolver, componentDependencyRegistry)); + configuration.add("Component", new ComponentWorker(resolver)); configuration.add("Mixin", new MixinWorker(resolver)); configuration.addInstance("InjectPage", InjectPageWorker.class); configuration.addInstance("InjectComponent", InjectComponentWorker.class); @@ -2792,7 +2802,7 @@ public final class TapestryModule componentDependencyRegistry.listen(componentTemplateSource.getInvalidationEventHub()); return componentDependencyRegistry; } - + private static final class TapestryCoreComponentLibraryInfoSource implements ComponentLibraryInfoSource { @@ -2813,8 +2823,8 @@ public final class TapestryModule info.setDescription("Components provided out-of-the-box by Tapestry"); info.setDocumentationUrl("http://tapestry.apache.org/component-reference.html";); info.setJavadocUrl("http://tapestry.apache.org/current/apidocs/";); - info.setSourceBrowseUrl("https://git-wip-us.apache.org/repos/asf?p=tapestry-5.git;a=summary";); - info.setSourceRootUrl("https://git-wip-us.apache.org/repos/asf?p=tapestry-5.git;a=blob;f=tapestry-core/src/main/java/";); + info.setSourceBrowseUrl("https://gitbox.apache.org/repos/asf?p=tapestry-5.git;a=summary";); + info.setSourceRootUrl("https://gitbox.apache.org/repos/asf?p=tapestry-5.git;a=blob;f=tapestry-core/src/main/java/";); info.setIssueTrackerUrl("https://issues.apache.org/jira/browse/TAP5";); info.setHomepageUrl("http://tapestry.apache.org";); info.setLibraryMapping(libraryMapping); 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 1b17f69a9..64bc37c30 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 @@ -20,7 +20,10 @@ import java.util.Objects; import java.util.Set; import org.apache.tapestry5.commons.services.PlasticProxyFactory; +import org.apache.tapestry5.internal.plastic.PlasticClassLoader; +import org.apache.tapestry5.internal.plastic.PlasticClassPool; import org.apache.tapestry5.plastic.PlasticManager; +import org.apache.tapestry5.plastic.PlasticUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,13 +48,15 @@ public class PageClassloaderContext private final PlasticManager plasticManager; private final PlasticProxyFactory proxyFactory; + + private PageClassloaderContext root; /** * Name of the <code>unknown</code> context (i.e. the one for controlled classes * without dependency information at the moment). */ public static final String UNKOWN_CONTEXT_NAME = "unknown"; - + public PageClassloaderContext(String name, PageClassloaderContext parent, Set<String> classNames, @@ -64,6 +69,66 @@ public class PageClassloaderContext this.plasticManager = plasticProxyFactory.getPlasticManager(); this.proxyFactory = plasticProxyFactory; children = new HashSet<>(); + if (plasticProxyFactory.getClassLoader() instanceof PlasticClassLoader) + { + final PlasticClassLoader plasticClassLoader = (PlasticClassLoader) plasticManager.getClassLoader(); + plasticClassLoader.setTag(name); + plasticClassLoader.setFilter(this::filter); + plasticClassLoader.setAlternativeClassloading(this::alternativeClassLoading); + if (parent != null) + { + getPlasticManager().getPool().setParent(parent.getPlasticManager().getPool()); + } + } + } + + private Class<?> alternativeClassLoading(String className) + { + Class<?> clasz = null; + setRootFieldIfNeeded(); + PageClassloaderContext context = root.findByClassName( + PlasticUtils.getEnclosingClassName(className)); + if (isRoot() && context == null) + { + context = this; + } + if (context != null) + { + try + { + final PlasticClassLoader classLoader = (PlasticClassLoader) context.getClassLoader(); + // Avoiding infinite recursion + synchronized (classLoader) + { + classLoader.setAlternativeClassloading(null); + clasz = classLoader.loadClass(className); + classLoader.setAlternativeClassloading(this::alternativeClassLoading); + } + } catch (ClassNotFoundException e) + { + throw new RuntimeException(e); + } + } + return clasz; + } + + private void setRootFieldIfNeeded() + { + if (root == null) + { + if (isRoot()) + { + root = this; + } + else + { + root = this; + while (!root.isRoot()) + { + root = root.getParent(); + } + } + } } /** @@ -181,7 +246,7 @@ public class PageClassloaderContext } LOGGER.debug("Invalidating page classloader context '{}' (class loader {}, classes : {})", name, proxyFactory.getClassLoader(), classNames); - classNames.clear(); +// classNames.clear(); parent.getChildren().remove(this); proxyFactory.clearCache(); } @@ -282,4 +347,16 @@ public class PageClassloaderContext } return string; } + + private boolean filter(String className) + { + final int index = className.indexOf("$"); + if (index > 0) + { + className = className.substring(0, index); + } + // TODO: do we really need the className.contains(".base.") part? + return classNames.contains(className) || className.contains(".base.") || isUnknown(); + } + } 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 8ead155d3..3d3ff1b20 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 @@ -17,8 +17,8 @@ import java.util.Set; import java.util.function.Function; import org.apache.tapestry5.commons.services.PlasticProxyFactory; -import org.apache.tapestry5.internal.services.ComponentDependencyRegistry; import org.apache.tapestry5.internal.services.ComponentInstantiatorSource; +import org.apache.tapestry5.ioc.annotations.UsesOrderedConfiguration; /** * Service that creates {@linkplain PageClassloaderContext} instances (except the root one) @@ -26,6 +26,7 @@ import org.apache.tapestry5.internal.services.ComponentInstantiatorSource; * contexts may be reused for a given class, specially when in production mode. * * @see ComponentInstantiatorSource + * @see DynamicDependenciesDefinition * @since 5.8.3 */ public interface PageClassloaderContextManager @@ -79,4 +80,12 @@ public interface PageClassloaderContextManager */ void initialize(PageClassloaderContext root, Function<ClassLoader, PlasticProxyFactory> plasticProxyFactoryProvider); + /** + * Returns the Class instance appropriate for a given component given a page name. + * @param clasz the class instance. + * @param pageName the page name. + * @return a Class instance. + */ + Class<?> getClassInstance(Class<?> clasz, String pageName); + } 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 94771139b..054129f44 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 @@ -25,12 +25,14 @@ import java.util.function.Supplier; import java.util.stream.Collectors; import org.apache.tapestry5.SymbolConstants; +import org.apache.tapestry5.commons.internal.util.TapestryException; import org.apache.tapestry5.commons.services.InvalidationEventHub; import org.apache.tapestry5.commons.services.PlasticProxyFactory; import org.apache.tapestry5.internal.services.ComponentDependencyRegistry; import org.apache.tapestry5.internal.services.InternalComponentInvalidationEventHub; import org.apache.tapestry5.ioc.annotations.ComponentClasses; import org.apache.tapestry5.ioc.annotations.Symbol; +import org.apache.tapestry5.plastic.PlasticUtils; import org.apache.tapestry5.services.ComponentClassResolver; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -111,7 +113,7 @@ public class PageClassloaderContextManagerImpl implements PageClassloaderContext else { - final String enclosingClassName = getEnclosingClassName(className); + final String enclosingClassName = PlasticUtils.getEnclosingClassName(className); context = root.findByClassName(enclosingClassName); if (context == null) @@ -132,7 +134,7 @@ public class PageClassloaderContextManagerImpl implements PageClassloaderContext if (!className.equals(enclosingClassName)) { - context.addClass(className); + loadClass(className, context); } } @@ -143,18 +145,10 @@ public class PageClassloaderContextManagerImpl implements PageClassloaderContext } - /** - * If the given class is an inner class, returns the enclosing class. - * Otherwise, returns the class name unchanged. - */ - private String getEnclosingClassName(String className) - { - int index = className.indexOf('$'); - return index <= 0 ? className : className.substring(0, index); - } - private PageClassloaderContext getUnknownContext(final PageClassloaderContext root, - final Function<ClassLoader, PlasticProxyFactory> plasticProxyFactoryProvider) { + final Function<ClassLoader, PlasticProxyFactory> plasticProxyFactoryProvider) + { + PageClassloaderContext unknownContext = null; for (PageClassloaderContext child : root.getChildren()) @@ -186,16 +180,29 @@ public class PageClassloaderContextManagerImpl implements PageClassloaderContext return processUsingDependencies(className, root, unknownContextProvider, plasticProxyFactoryProvider, classesToInvalidate, new HashSet<>()); } - private PageClassloaderContext processUsingDependencies( String className, PageClassloaderContext root, Supplier<PageClassloaderContext> unknownContextProvider, - Function<ClassLoader, PlasticProxyFactory> plasticProxyFactoryProvider, Set<String> classesToInvalidate, + Function<ClassLoader, PlasticProxyFactory> plasticProxyFactoryProvider, + Set<String> classesToInvalidate, Set<String> alreadyProcessed) + { + return processUsingDependencies(className, root, unknownContextProvider, + plasticProxyFactoryProvider, classesToInvalidate, alreadyProcessed, true); + } + + + private PageClassloaderContext processUsingDependencies( + String className, + PageClassloaderContext root, + Supplier<PageClassloaderContext> unknownContextProvider, + Function<ClassLoader, PlasticProxyFactory> plasticProxyFactoryProvider, + Set<String> classesToInvalidate, + Set<String> alreadyProcessed, + boolean processCircularDependencies) { PageClassloaderContext context = root.findByClassName(className); - Set<String> circularDependencies = new HashSet<>(1); if (context == null) { @@ -209,18 +216,23 @@ public class PageClassloaderContextManagerImpl implements PageClassloaderContext !componentDependencyRegistry.contains(className) && componentDependencyRegistry.getDependents(className).isEmpty()) { - // Make sure you get a fresh version of the class before processing its - // dependencies - Class<?> clasz; - PlasticProxyFactory throwaway = plasticProxyFactoryProvider.apply(root.getClassLoader()); - try { - clasz = throwaway.getClassLoader().loadClass(className); - } catch (ClassNotFoundException e) { - throw new RuntimeException(e); - } - componentDependencyRegistry.register(clasz); - alreadyProcessed.remove(className); - return processUsingDependencies(className, root, unknownContextProvider, plasticProxyFactoryProvider, classesToInvalidate, alreadyProcessed); + context = unknownContextProvider.get(); +// // Make sure you get a fresh version of the class before processing its +// // dependencies +// PageClassloaderContext throwaway = new PageClassloaderContext( +// "Throwaway", +// root, +// Collections.singleton(className), +// plasticProxyFactoryProvider.apply(root.getClassLoader())); +// +// Class<?> clasz = loadClass(className, throwaway); +// componentDependencyRegistry.register(clasz); +// +// throwaway.getProxyFactory().clearCache(); +// throwaway.getClassNames().clear(); +// +// alreadyProcessed.remove(className); +// return processUsingDependencies(className, root, unknownContextProvider, plasticProxyFactoryProvider, classesToInvalidate, alreadyProcessed); } else { @@ -228,17 +240,18 @@ public class PageClassloaderContextManagerImpl implements PageClassloaderContext alreadyProcessed.add(className); // Sorting dependencies alphabetically so we have consistent results. - List<String> dependencies = new ArrayList<>(componentDependencyRegistry.getDependencies(className)); + List<String> dependencies = new ArrayList<>(getDependenciesWithoutPages(className)); Collections.sort(dependencies); // Process dependencies depth-first for (String dependency : dependencies) { // Avoid infinite recursion loops - if (!alreadyProcessed.contains(dependency)) + if (!alreadyProcessed.contains(dependency)/* && + !circularDependencies.contains(dependency)*/) { processUsingDependencies(dependency, root, unknownContextProvider, - plasticProxyFactoryProvider, classesToInvalidate, alreadyProcessed); + plasticProxyFactoryProvider, classesToInvalidate, alreadyProcessed, false); } } @@ -246,13 +259,9 @@ public class PageClassloaderContextManagerImpl implements PageClassloaderContext Set<PageClassloaderContext> contextDependencies = new HashSet<>(); for (String dependency : dependencies) { - // Direct circular dependency - if (componentDependencyRegistry.getDependencies(dependency).contains(className)) - { - circularDependencies.add(dependency); - } - else - { + // Circular dependency +// if (!circularDependencies.contains(dependency)) +// { PageClassloaderContext dependencyContext = root.findByClassName(dependency); if (dependencyContext == null) { @@ -261,7 +270,7 @@ public class PageClassloaderContextManagerImpl implements PageClassloaderContext } contextDependencies.add(dependencyContext); - } +// } } if (contextDependencies.size() == 0) @@ -287,10 +296,19 @@ public class PageClassloaderContextManagerImpl implements PageClassloaderContext getContextName(className), parentContext, Collections.singleton(className), - plasticProxyFactoryProvider.apply(root.getClassLoader())); + plasticProxyFactoryProvider.apply(parentContext.getClassLoader())); } 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()*/) + { + loadClass(className, context); + } LOGGER.debug("New context: {}", context); @@ -299,11 +317,64 @@ public class PageClassloaderContextManagerImpl implements PageClassloaderContext } context.addClass(className); - for (String circularDependency : circularDependencies) + + // Merge contexts of circular dependencies into a single one + +// if (processCircularDependencies && !circularDependencies.isEmpty()) +// { +// Set<PageClassloaderContext> contexts = new HashSet<>(circularDependencies.size() + 1); +// contexts.add(context); +// for (String circularDependency : circularDependencies) +// { +// PageClassloaderContext toMerge = +// processUsingDependencies( +// circularDependency, root, unknownContextProvider, +// plasticProxyFactoryProvider, classesToInvalidate, alreadyProcessed, +// false); +// contexts.add(toMerge); +// } +// if (contexts.size() > 1) +// { +// +// context = merge(contexts, plasticProxyFactoryProvider, root, classesToInvalidate); +// +// // Avoiding an infinite recursion +// classesToInvalidate.removeAll(context.getClassNames()); +// +// // Ensure non-page class is initialized in the correct context and classloader +// for (String clasz : context.getClassNames()) { +// if (!componentClassResolver.isPage(clasz)) +// { +// loadClass(clasz, context); +// } +// } +// } +// } + return context; + } + + private Set<String> getDependenciesWithoutPages(String className) { + final Set<String> dependencies = componentDependencyRegistry.getDependencies(className); + return dependencies.stream() + .filter(c -> !componentClassResolver.isPage(c)) + .collect(Collectors.toSet()); + } + + private Class<?> loadClass(String className, PageClassloaderContext context) + { + try { - context.addClass(circularDependency); + final ClassLoader classLoader = context.getPlasticManager().getClassLoader(); + final Class<?> clasz = classLoader.loadClass(className); + // TODO: do were really need this loop? + for (Class<?> c : clasz.getDeclaredClasses()) + { + final Class<?> cccc = classLoader.loadClass(c.getName()); + } + return clasz; + } catch (Exception e) { + throw new RuntimeException(e); } - return context; } private PageClassloaderContext merge( @@ -403,6 +474,11 @@ public class PageClassloaderContextManagerImpl implements PageClassloaderContext parent.addChild(merged); +// for (String className : classNames) +// { +// loadClass(className, merged); +// } + NESTED_MERGE_COUNT.set(NESTED_MERGE_COUNT.get() - 1); if (LOGGER.isDebugEnabled()) { @@ -488,6 +564,7 @@ public class PageClassloaderContextManagerImpl implements PageClassloaderContext LOGGER.debug("Invalidating classes {}", classesToInvalidate); INVALIDATING_CONTEXT.set(true); final ArrayList<String> classesToInvalidateAsList = new ArrayList<>(classesToInvalidate); + // TODO: do we really need both invalidation hubs to be invoked here? invalidationHub.fireInvalidationEvent(classesToInvalidateAsList); componentClassesInvalidationEventHub.fireInvalidationEvent(classesToInvalidateAsList); INVALIDATING_CONTEXT.set(false); @@ -517,4 +594,45 @@ public class PageClassloaderContextManagerImpl implements PageClassloaderContext { } + @Override + public Class<?> getClassInstance(Class<?> clasz, String pageName) + { + final String className = clasz.getName(); + PageClassloaderContext context = root.findByClassName(className); + if (context == null) + { + context = get(className); + } +// final String pageClassName = componentClassResolver.resolvePageNameToClassName(pageName); +// final PageClassloaderContext context = get(pageClassName); +// PageClassloaderContext ancestor = context.getParent(); +// +// if (!className.equals(pageClassName)) +// { +// +// while (ancestor != null && !ancestor.getClassNames().contains(className)) +// { +// ancestor = ancestor.getParent(); +// } +// +// if (ancestor == null) +// { +// ancestor = context; +// } +// +// if (clasz.getSimpleName().equals("TextField")) +// { +// System.out.printf("XXXXX Target class (before): %s page %s context : %s\n", clasz.getSimpleName(), pageName, clasz.getClassLoader()); +// } + try + { + clasz = context.getProxyFactory().getClassLoader().loadClass(className); + } catch (ClassNotFoundException e) + { + throw new TapestryException(e.getMessage(), e); + } +// } + return clasz; + } + } diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/services/AppModule.java b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/services/AppModule.java index c9b5b7505..5f57836bf 100644 --- a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/services/AppModule.java +++ b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/services/AppModule.java @@ -139,7 +139,7 @@ public class AppModule { long elapsed = System.nanoTime() - startTime; -// log.info(String.format("Request time: %5.2f s -- %s", elapsed * 10E-10d, request.getPath())); + log.info(String.format("Request time: %5.2f s -- %s", elapsed * 10E-10d, request.getPath())); } } }; @@ -395,9 +395,9 @@ public class AppModule @Contribute(PagePreloader.class) public static void setupPagePreload(Configuration<String> configuration) { -// configuration.add("index"); -// configuration.add("core/exceptionreport"); -// configuration.add("core/t5dashboard"); + configuration.add("index"); + configuration.add("core/exceptionreport"); + configuration.add("core/t5dashboard"); } diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ComponentDependencyRegistryImplTest.java b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ComponentDependencyRegistryImplTest.java index 847494f96..fcb776d6a 100644 --- a/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ComponentDependencyRegistryImplTest.java +++ b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ComponentDependencyRegistryImplTest.java @@ -33,15 +33,35 @@ import org.apache.tapestry5.commons.MappedConfiguration; import org.apache.tapestry5.corelib.base.AbstractComponentEventLink; import org.apache.tapestry5.corelib.base.AbstractField; import org.apache.tapestry5.corelib.base.AbstractLink; +import org.apache.tapestry5.corelib.base.AbstractPropertyOutput; import org.apache.tapestry5.corelib.base.AbstractTextField; import org.apache.tapestry5.corelib.components.ActionLink; +import org.apache.tapestry5.corelib.components.Alerts; import org.apache.tapestry5.corelib.components.Any; import org.apache.tapestry5.corelib.components.BeanEditForm; +import org.apache.tapestry5.corelib.components.BeanEditor; +import org.apache.tapestry5.corelib.components.Delegate; +import org.apache.tapestry5.corelib.components.DevTool; +import org.apache.tapestry5.corelib.components.Errors; import org.apache.tapestry5.corelib.components.EventLink; +import org.apache.tapestry5.corelib.components.Form; +import org.apache.tapestry5.corelib.components.Glyphicon; import org.apache.tapestry5.corelib.components.If; +import org.apache.tapestry5.corelib.components.Label; +import org.apache.tapestry5.corelib.components.Loop; +import org.apache.tapestry5.corelib.components.Output; +import org.apache.tapestry5.corelib.components.PageLink; +import org.apache.tapestry5.corelib.components.PropertyDisplay; +import org.apache.tapestry5.corelib.components.PropertyEditor; +import org.apache.tapestry5.corelib.components.RenderObject; +import org.apache.tapestry5.corelib.components.Submit; import org.apache.tapestry5.corelib.components.TextField; +import org.apache.tapestry5.corelib.components.TextOutput; import org.apache.tapestry5.corelib.components.Zone; +import org.apache.tapestry5.corelib.mixins.FormGroup; import org.apache.tapestry5.corelib.mixins.RenderDisabled; +import org.apache.tapestry5.corelib.pages.PropertyDisplayBlocks; +import org.apache.tapestry5.corelib.pages.PropertyEditBlocks; import org.apache.tapestry5.integration.app1.components.Border; import org.apache.tapestry5.integration.app1.components.ErrorComponent; import org.apache.tapestry5.integration.app1.components.OuterAny; @@ -51,6 +71,7 @@ import org.apache.tapestry5.integration.app1.mixins.EchoValue; import org.apache.tapestry5.integration.app1.mixins.EchoValue2; import org.apache.tapestry5.integration.app1.mixins.TextOnlyOnDisabled; import org.apache.tapestry5.integration.app1.pages.AlertsDemo; +import org.apache.tapestry5.integration.app1.pages.BeanEditorWithFormFragmentDemo; import org.apache.tapestry5.integration.app1.pages.BlockCaller; import org.apache.tapestry5.integration.app1.pages.BlockHolder; import org.apache.tapestry5.integration.app1.pages.EmbeddedComponentTypeConflict; @@ -109,6 +130,22 @@ public class ComponentDependencyRegistryImplTest expectResolveComponent(If.class); expectResolveComponent(ErrorComponent.class); expectResolveComponent(EventLink.class); + expectResolveComponent(Output.class); + expectResolveComponent(Delegate.class); + expectResolveComponent(TextOutput.class); + expectResolveComponent(Label.class); + expectResolveComponent(BeanEditor.class); + expectResolveComponent(Loop.class); + expectResolveComponent(PropertyEditor.class); + expectResolveComponent(PropertyDisplay.class); + expectResolveComponent(Errors.class); + expectResolveComponent(Submit.class); + expectResolveComponent(PageLink.class); + expectResolveComponent(DevTool.class); + expectResolveComponent(Alerts.class); + expectResolveComponent(RenderObject.class); + expectResolveComponent(Form.class); + expectResolveComponent(Glyphicon.class); EasyMock.expect(resolver.resolveMixinTypeToClassName("textonlyondisabled")) .andReturn(TextOnlyOnDisabled.class.getName()).anyTimes(); @@ -116,6 +153,15 @@ public class ComponentDependencyRegistryImplTest .andReturn(EchoValue2.class.getName()).anyTimes(); EasyMock.expect(resolver.resolveMixinTypeToClassName("alttitledefault")) .andReturn(AltTitleDefault.class.getName()).anyTimes(); + EasyMock.expect(resolver.resolveMixinTypeToClassName("formgroup")) + .andReturn(FormGroup.class.getName()).anyTimes(); + + // TODO: remove this +// EasyMock.expect(resolver.getLogicalName(EasyMock.anyString())).andAnswer(() -> (String) EasyMock.getCurrentArguments()[0]).anyTimes(); + EasyMock.expect(resolver.isPage(EasyMock.anyString())).andAnswer(() -> { + String string = (String) EasyMock.getCurrentArguments()[0]; + return string.contains(".pages."); + }).anyTimes(); pageClassloaderContextManager = EasyMock.createMock(PageClassloaderContextManager.class); plasticManager = EasyMock.createMock(PlasticManager.class); @@ -125,16 +171,23 @@ public class ComponentDependencyRegistryImplTest return className.contains(".pages.") || className.contains(".mixins.") || className.contains(".components.") || className.contains(".base."); }).anyTimes(); + componentDependencyRegistry = new ComponentDependencyRegistryImpl( - pageClassloaderContextManager, plasticManager, resolver, templateParser, componentTemplateLocator); + pageClassloaderContextManager, plasticManager, resolver, templateParser, + componentTemplateLocator); EasyMock.replay(pageClassloaderContextManager, plasticManager, resolver); } private void expectResolveComponent(final Class<?> clasz) { - EasyMock.expect(resolver.resolveComponentTypeToClassName(clasz.getSimpleName())) - .andReturn(clasz.getName()).anyTimes(); - EasyMock.expect(resolver.resolveComponentTypeToClassName(clasz.getSimpleName().toLowerCase())) - .andReturn(clasz.getName()).anyTimes(); + final String className = clasz.getName(); + final java.lang.String simpleName = clasz.getSimpleName(); + EasyMock.expect(resolver.resolveComponentTypeToClassName(simpleName)) + .andReturn(className).anyTimes(); + EasyMock.expect(resolver.resolveComponentTypeToClassName(simpleName.toLowerCase())) + .andReturn(className).anyTimes(); + EasyMock.expect(resolver.resolveComponentTypeToClassName( + simpleName.substring(0, 1).toLowerCase() + simpleName.substring(1))) + .andReturn(className).anyTimes(); } private void configurePCCM(boolean merging) @@ -240,6 +293,14 @@ public class ComponentDependencyRegistryImplTest componentDependencyRegistry.clear(); + // Dynamic dependency definitions + componentDependencyRegistry.register(PropertyDisplay.class); + assertDependencies(PropertyDisplay.class, + PropertyDisplayBlocks.class, AbstractPropertyOutput.class); + + componentDependencyRegistry.register(PropertyEditor.class); + assertDependencies(PropertyEditor.class, PropertyEditBlocks.class); + // Superclass componentDependencyRegistry.register(EventLink.class); @@ -290,6 +351,32 @@ public class ComponentDependencyRegistryImplTest } + + @Test + public void getCircularDependencies() + { +// componentDependencyRegistry.clear(); +// componentDependencyRegistry.register(BeanEditor.class); +// Set<String> expected = setOf(PropertyEditor.class, PropertyEditBlocks.class); +// assertEquals(expected, +// componentDependencyRegistry.getCircularDependencies(BeanEditor.class.getName())); +// +// componentDependencyRegistry.register(BlockCaller.class); +// assertEquals(setOf(BlockHolder.class), +// componentDependencyRegistry.getCircularDependencies(BlockCaller.class.getName())); +// assertEquals(setOf(BlockCaller.class), +// componentDependencyRegistry.getCircularDependencies(BlockHolder.class.getName())); +// +// // An infinite recursion happened in this call. +// componentDependencyRegistry.register(Index.class); +// assertEquals(new HashSet<String>(), +// componentDependencyRegistry.getCircularDependencies(Delegate.class.getName())); + + componentDependencyRegistry.register(BeanEditorWithFormFragmentDemo.class); + + ComponentDependencyGraphvizGenerator generator = new ComponentDependencyGraphvizGeneratorImpl(componentDependencyRegistry, resolver); + System.out.println(generator.generate(BeanEditorWithFormFragmentDemo.class.getName())); + } private void assertDependencies(Class clasz, Class... dependencies) { assertEquals( diff --git a/tapestry-core/src/test/resources/log4j.properties b/tapestry-core/src/test/resources/log4j.properties index 6e07ce841..f9124f576 100644 --- a/tapestry-core/src/test/resources/log4j.properties +++ b/tapestry-core/src/test/resources/log4j.properties @@ -8,8 +8,6 @@ log4j.appender.A1.layout=org.apache.log4j.PatternLayout log4j.appender.A1.layout.ConversionPattern=%t [%p] %c{1} %m%n log4j.category.org.apache.tapestry5.integration.app2=debug -log4j.category.org.apache.tapestry5.services.pageload.PageClassloaderContextManager=debug -log4j.category.org.apache.tapestry5.services.pageload.PageClassloaderContext=debug # log4j.category.tapestry.render=debug @@ -22,5 +20,7 @@ log4j.category.org.openqa.selenium.server=warn log4j.category.org.apache.tapestry5.integration.app1.base.InheritBase=debug log4j.category.org.apache.tapestry5.integration.app1.pages.inherit=debug +log4j.category.org.apache.tapestry5.services.pageload=debug +log4j.category.org.apache.tapestry5.integration.app1.services.AppModule.TimingFilter=ERROR # log4j.category.tapestry.transformer.org.apache.tapestry5.integration.app1.base.InheritBase=debug # log4j.category.tapestry.transformer.org.apache.tapestry5.integration.app1.pages.inherit=debug
