This is an automated email from the ASF dual-hosted git repository. thiagohp pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tapestry-5.git
The following commit(s) were added to refs/heads/master by this push: new 49f9fdc5f When a non proxied object is beeing eager loaded registry has to instantiate all its constructor parameters. (#35) 49f9fdc5f is described below commit 49f9fdc5f5e9ba820d86f92d32e2a45221753ebd Author: zenios <dimitris.zen...@gmail.com> AuthorDate: Tue Aug 23 15:08:34 2022 +0300 When a non proxied object is beeing eager loaded registry has to instantiate all its constructor parameters. (#35) If one of these parameters is an eager loaded services (Not yet loaded) registry will create a proxy for it. Because the proxy is created, that eager load service will not be eager loaded since its already registered as a realized service. These issue is related with the following 1. https://dev.tapestry.apache.narkive.com/m806StSk/strange-behavour-of-eagerload 2. https://issues.apache.org/jira/browse/TAPESTRY-226. --- .../apache/tapestry5/ioc/internal/ModuleImpl.java | 33 +++++++++++++--------- .../src/test/groovy/ioc/specs/EagerLoadSpec.groovy | 8 ++++-- ...agerLoadService.java => EagerLoadService1.java} | 2 +- ...ServiceImpl.java => EagerLoadService1Impl.java} | 6 ++-- ...agerLoadService.java => EagerLoadService2.java} | 2 +- ...ServiceImpl.java => EagerLoadService2Impl.java} | 8 ++---- .../tapestry5/ioc/test/EagerProxyReloadModule.java | 9 ++++-- .../ioc/test/NonProxiedEagerLoadService.java | 7 +++++ 8 files changed, 48 insertions(+), 27 deletions(-) diff --git a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ModuleImpl.java b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ModuleImpl.java index c2233c3f9..da90942f8 100644 --- a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ModuleImpl.java +++ b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ModuleImpl.java @@ -116,6 +116,12 @@ public class ModuleImpl implements Module */ private final Map<String, Object> services = CollectionFactory.newCaseInsensitiveMap(); + + /** + * EagerLoadProxies collection into which proxies for eager loaded services are added. Guarded by BARRIER + */ + private final Collection<EagerLoadServiceProxy> eagerLoadProxies = CollectionFactory.newList(); + private final Map<String, ServiceDef3> serviceDefs = CollectionFactory.newCaseInsensitiveMap(); /** @@ -161,7 +167,7 @@ public class ModuleImpl implements Module // RegistryImpl should already have checked that the service exists. assert def != null; - Object service = findOrCreate(def, null); + Object service = findOrCreate(def); try { @@ -225,10 +231,9 @@ public class ModuleImpl implements Module * Locates the service proxy for a particular service (from the service definition). * * @param def defines the service - * @param eagerLoadProxies collection into which proxies for eager loaded services are added (or null) * @return the service proxy */ - private Object findOrCreate(final ServiceDef3 def, final Collection<EagerLoadServiceProxy> eagerLoadProxies) + private Object findOrCreate(final ServiceDef3 def) { final String key = def.getServiceId(); @@ -247,7 +252,7 @@ public class ModuleImpl implements Module if (result == null) { - result = create(def, eagerLoadProxies); + result = create(def); services.put(key, result); } @@ -284,8 +289,16 @@ public class ModuleImpl implements Module for (ServiceDef3 def : serviceDefs.values()) { if (def.isEagerLoad()) - findOrCreate(def, proxies); + findOrCreate(def); } + + BARRIER.withWrite(new Runnable() { + @Override + public void run() { + proxies.addAll(eagerLoadProxies); + eagerLoadProxies.clear(); + } + }); } }; @@ -294,10 +307,8 @@ public class ModuleImpl implements Module /** * Creates the service and updates the cache of created services. - * - * @param eagerLoadProxies a list into which any eager loaded proxies should be added */ - private Object create(final ServiceDef3 def, final Collection<EagerLoadServiceProxy> eagerLoadProxies) + private Object create(final ServiceDef3 def) { final String serviceId = def.getServiceId(); @@ -376,11 +387,7 @@ public class ModuleImpl implements Module registry.addRegistryShutdownListener(delegate); - // Occasionally eager load service A may invoke service B from its service builder method; if - // service B is eager loaded, we'll hit this method but eagerLoadProxies will be null. That's OK - // ... service B is being realized anyway. - - if (def.isEagerLoad() && eagerLoadProxies != null) + if (def.isEagerLoad()) eagerLoadProxies.add(delegate); tracker.setStatus(serviceId, Status.VIRTUAL); diff --git a/tapestry-ioc/src/test/groovy/ioc/specs/EagerLoadSpec.groovy b/tapestry-ioc/src/test/groovy/ioc/specs/EagerLoadSpec.groovy index de47abe2b..73e81b7d8 100644 --- a/tapestry-ioc/src/test/groovy/ioc/specs/EagerLoadSpec.groovy +++ b/tapestry-ioc/src/test/groovy/ioc/specs/EagerLoadSpec.groovy @@ -7,7 +7,9 @@ class EagerLoadSpec extends AbstractRegistrySpecification { def "proxied service does eager load"() { expect: - EagerProxyReloadModule.eagerLoadServiceDidLoad == false + EagerProxyReloadModule.eagerLoadService1DidLoad == false + EagerProxyReloadModule.nonProxyEagerLoadServiceDidLoad == false + EagerProxyReloadModule.eagerLoadService2DidLoad == false when: @@ -17,6 +19,8 @@ class EagerLoadSpec extends AbstractRegistrySpecification { then: - EagerProxyReloadModule.eagerLoadServiceDidLoad == true + EagerProxyReloadModule.eagerLoadService1DidLoad == true + EagerProxyReloadModule.nonProxyEagerLoadServiceDidLoad == true + EagerProxyReloadModule.eagerLoadService2DidLoad == true } } diff --git a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService.java b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService1.java similarity index 94% copy from tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService.java copy to tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService1.java index 808fb8c36..2cce5732c 100644 --- a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService.java +++ b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService1.java @@ -14,7 +14,7 @@ package org.apache.tapestry5.ioc.test; -public interface EagerLoadService +public interface EagerLoadService1 { } diff --git a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadServiceImpl.java b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService1Impl.java similarity index 81% copy from tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadServiceImpl.java copy to tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService1Impl.java index d2274d0c5..8217db323 100644 --- a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadServiceImpl.java +++ b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService1Impl.java @@ -17,10 +17,10 @@ package org.apache.tapestry5.ioc.test; import org.apache.tapestry5.ioc.annotations.EagerLoad; @EagerLoad -public class EagerLoadServiceImpl implements EagerLoadService +public class EagerLoadService1Impl implements EagerLoadService1 { - public EagerLoadServiceImpl() + public EagerLoadService1Impl() { - EagerProxyReloadModule.eagerLoadServiceDidLoad = true; + EagerProxyReloadModule.eagerLoadService1DidLoad = true; } } diff --git a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService.java b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService2.java similarity index 94% rename from tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService.java rename to tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService2.java index 808fb8c36..020982502 100644 --- a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService.java +++ b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService2.java @@ -14,7 +14,7 @@ package org.apache.tapestry5.ioc.test; -public interface EagerLoadService +public interface EagerLoadService2 { } diff --git a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadServiceImpl.java b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService2Impl.java similarity index 74% rename from tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadServiceImpl.java rename to tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService2Impl.java index d2274d0c5..24f60a84a 100644 --- a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadServiceImpl.java +++ b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerLoadService2Impl.java @@ -14,13 +14,11 @@ package org.apache.tapestry5.ioc.test; -import org.apache.tapestry5.ioc.annotations.EagerLoad; -@EagerLoad -public class EagerLoadServiceImpl implements EagerLoadService +public class EagerLoadService2Impl implements EagerLoadService2 { - public EagerLoadServiceImpl() + public EagerLoadService2Impl() { - EagerProxyReloadModule.eagerLoadServiceDidLoad = true; + EagerProxyReloadModule.eagerLoadService2DidLoad = true; } } diff --git a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerProxyReloadModule.java b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerProxyReloadModule.java index 1ba7d39f9..5375fa7c1 100644 --- a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerProxyReloadModule.java +++ b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/EagerProxyReloadModule.java @@ -16,12 +16,17 @@ package org.apache.tapestry5.ioc.test; import org.apache.tapestry5.ioc.ServiceBinder; +//@ImportModule(EagerProxy2ReloadModule.class) public class EagerProxyReloadModule { - public static boolean eagerLoadServiceDidLoad; + public static boolean eagerLoadService1DidLoad; + public static boolean nonProxyEagerLoadServiceDidLoad; + public static boolean eagerLoadService2DidLoad; public static void bind(ServiceBinder binder) { - binder.bind(EagerLoadService.class, EagerLoadServiceImpl.class); + binder.bind(EagerLoadService1.class, EagerLoadService1Impl.class); + binder.bind(NonProxiedEagerLoadService.class).eagerLoad(); + binder.bind(EagerLoadService2.class, EagerLoadService2Impl.class).eagerLoad(); } } diff --git a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/NonProxiedEagerLoadService.java b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/NonProxiedEagerLoadService.java new file mode 100644 index 000000000..60d716da1 --- /dev/null +++ b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/test/NonProxiedEagerLoadService.java @@ -0,0 +1,7 @@ +package org.apache.tapestry5.ioc.test; + +public class NonProxiedEagerLoadService { + public NonProxiedEagerLoadService(EagerLoadService2 service2) { + EagerProxyReloadModule.nonProxyEagerLoadServiceDidLoad = true; + } +}