This is an automated email from the ASF dual-hosted git repository. arne pushed a commit to branch OWB-1393 in repository https://gitbox.apache.org/repos/asf/openwebbeans.git
commit de7a9ad7256208349aad26f22f4c2fa1fd9eca20 Author: arne <[email protected]> AuthorDate: Sun Oct 10 12:23:52 2021 +0200 [OWB-1393] Don't fire ProcessObserverMethod during extension registration --- .../apache/webbeans/el/test/AbstractUnitTest.java | 5 +- .../component/creation/ExtensionBeanBuilder.java | 18 +++- .../component/creation/ObserverMethodsBuilder.java | 98 ++++++++++++----- .../apache/webbeans/event/NotificationManager.java | 28 +++-- .../webbeans/portable/events/ExtensionLoader.java | 116 ++++++++++++--------- .../org/apache/webbeans/test/AbstractUnitTest.java | 5 +- .../portable/events/ProcessObserverMethodTest.java | 58 +++++++++++ .../extensions/ProcessObserverMethodExtension.java | 42 ++++++++ 8 files changed, 279 insertions(+), 91 deletions(-) diff --git a/webbeans-el22/src/test/java/org/apache/webbeans/el/test/AbstractUnitTest.java b/webbeans-el22/src/test/java/org/apache/webbeans/el/test/AbstractUnitTest.java index 211f4d2..730afa0 100644 --- a/webbeans-el22/src/test/java/org/apache/webbeans/el/test/AbstractUnitTest.java +++ b/webbeans-el22/src/test/java/org/apache/webbeans/el/test/AbstractUnitTest.java @@ -70,10 +70,7 @@ public abstract class AbstractUnitTest }; webBeansContext = WebBeansContext.getInstance(); - for (Extension ext : extensions) - { - webBeansContext.getExtensionLoader().addExtension(ext); - } + webBeansContext.getExtensionLoader().addExtensions(extensions); //Deploy bean classes OpenWebBeansTestMetaDataDiscoveryService discoveryService = (OpenWebBeansTestMetaDataDiscoveryService)webBeansContext.getScannerService(); diff --git a/webbeans-impl/src/main/java/org/apache/webbeans/component/creation/ExtensionBeanBuilder.java b/webbeans-impl/src/main/java/org/apache/webbeans/component/creation/ExtensionBeanBuilder.java index 93f8e26..6e8ddf0 100644 --- a/webbeans-impl/src/main/java/org/apache/webbeans/component/creation/ExtensionBeanBuilder.java +++ b/webbeans-impl/src/main/java/org/apache/webbeans/component/creation/ExtensionBeanBuilder.java @@ -19,22 +19,25 @@ package org.apache.webbeans.component.creation; import javax.enterprise.inject.spi.AnnotatedType; +import javax.enterprise.inject.spi.Extension; import org.apache.webbeans.component.ExtensionBean; import org.apache.webbeans.config.WebBeansContext; import org.apache.webbeans.util.Asserts; -public class ExtensionBeanBuilder<T> +public class ExtensionBeanBuilder<T extends Extension> { protected final WebBeansContext webBeansContext; protected final AnnotatedType<T> annotatedType; + protected final T extension; - public ExtensionBeanBuilder(WebBeansContext webBeansContext, Class<T> type) + public ExtensionBeanBuilder(WebBeansContext webBeansContext, T extension) { Asserts.assertNotNull(webBeansContext, Asserts.PARAM_NAME_WEBBEANSCONTEXT); - Asserts.assertNotNull(type, "type"); + Asserts.assertNotNull(extension, "extension"); this.webBeansContext = webBeansContext; - annotatedType = webBeansContext.getAnnotatedElementFactory().newAnnotatedType(type); + annotatedType = webBeansContext.getAnnotatedElementFactory().newAnnotatedType((Class<T>)extension.getClass()); + this.extension = extension; } public AnnotatedType<T> getAnnotatedType() @@ -42,7 +45,12 @@ public class ExtensionBeanBuilder<T> return annotatedType; } - public ExtensionBean<T> getBean() + public T getExtension() + { + return extension; + } + + public ExtensionBean<T> buildBean() { return new ExtensionBean<>(webBeansContext, annotatedType.getJavaClass()); } diff --git a/webbeans-impl/src/main/java/org/apache/webbeans/component/creation/ObserverMethodsBuilder.java b/webbeans-impl/src/main/java/org/apache/webbeans/component/creation/ObserverMethodsBuilder.java index 56e511d..7fea99b 100644 --- a/webbeans-impl/src/main/java/org/apache/webbeans/component/creation/ObserverMethodsBuilder.java +++ b/webbeans-impl/src/main/java/org/apache/webbeans/component/creation/ObserverMethodsBuilder.java @@ -66,11 +66,28 @@ public class ObserverMethodsBuilder<T> this.annotatedType = annotatedType; } + public Set<ObserverMethod<?>> defineContainerLifecycleEventObserverMethods(AbstractOwbBean<T> ownerBean) + { + Set<ObserverMethod<?>> definedObservers = new HashSet<>(); + for (AnnotatedMethod<?> annotatedMethod : webBeansContext.getAnnotatedElementFactory().getFilteredAnnotatedMethods(annotatedType)) + { + ObserverMethod<?> observerMethod = defineContainerLifecycleEventObserverMethod(ownerBean, annotatedMethod); + if (observerMethod != null) + { + definedObservers.add(observerMethod); + } + } + + checkDefinedObservers(ownerBean, definedObservers); + + return definedObservers; + } + /** * {@inheritDoc} */ public Set<ObserverMethod<?>> defineObserverMethods(AbstractOwbBean<T> ownerBean) - { + { Set<ObserverMethod<?>> definedObservers = new HashSet<>(); for (AnnotatedMethod<?> annotatedMethod : webBeansContext.getAnnotatedElementFactory().getFilteredAnnotatedMethods(annotatedType)) { @@ -81,19 +98,7 @@ public class ObserverMethodsBuilder<T> } } - if (!definedObservers.isEmpty()) - { - for (InjectionPoint ip : ownerBean.getInjectionPoints()) - { - Set<Annotation> qualifiers = ip.getQualifiers(); - if (EventMetadata.class == ip.getType() - && qualifiers != null && ip.getQualifiers().size() == 1 - && Default.class == qualifiers.iterator().next().annotationType()) - { - throw new WebBeansConfigurationException(ip + " is not an observer parameter"); - } - } - } + checkDefinedObservers(ownerBean, definedObservers); return definedObservers; } @@ -102,8 +107,48 @@ public class ObserverMethodsBuilder<T> * Check whether the given annotatedMethod is an ObserverMethod and verify it * @return the ObserverMethod or {@code null} if this method is not an observer. */ + public ObserverMethod<?> defineContainerLifecycleEventObserverMethod(AbstractOwbBean<T> ownerBean, AnnotatedMethod<?> annotatedMethod) + { + AnnotatedParameter<?> observesParameter = findObservesParameter(annotatedMethod); + + if (observesParameter != null) + { + checkObserverMethodConditions(ownerBean, observesParameter); + + //Looking for ObserverMethod + ObserverMethod<?> definedObserver = webBeansContext.getNotificationManager(). + getContainerLifecycleEventObservableMethodForAnnotatedMethod(annotatedMethod, observesParameter, ownerBean); + + return definedObserver; + } + + return null; + } + + /** + * Check whether the given annotatedMethod is an ObserverMethod and verify it + * @return the ObserverMethod or {@code null} if this method is not an observer. + */ public ObserverMethod<?> defineObserverMethod(AbstractOwbBean<T> ownerBean, AnnotatedMethod<?> annotatedMethod) { + AnnotatedParameter<?> observesParameter = findObservesParameter(annotatedMethod); + + if (observesParameter != null) + { + checkObserverMethodConditions(ownerBean, observesParameter); + + //Looking for ObserverMethod + ObserverMethod<?> definedObserver = webBeansContext.getNotificationManager(). + getObservableMethodForAnnotatedMethod(annotatedMethod, observesParameter, ownerBean); + + return definedObserver; + } + + return null; + } + + private AnnotatedParameter<?> findObservesParameter(AnnotatedMethod<?> annotatedMethod) + { List<AnnotatedParameter<?>> parameters = (List<AnnotatedParameter<?>>)(List<?>)annotatedMethod.getParameters(); AnnotatedParameter<?> observesParameter = null; for (AnnotatedParameter<?> parameter : parameters) @@ -119,19 +164,24 @@ public class ObserverMethodsBuilder<T> observesParameter = parameter; } } + return observesParameter; + } - if (observesParameter != null) + private void checkDefinedObservers(AbstractOwbBean<T> ownerBean, Set<ObserverMethod<?>> definedObservers) + { + if (!definedObservers.isEmpty()) { - checkObserverMethodConditions(ownerBean, observesParameter); - - //Looking for ObserverMethod - ObserverMethod<?> definedObserver = webBeansContext.getNotificationManager(). - getObservableMethodForAnnotatedMethod(annotatedMethod, observesParameter, ownerBean); - - return definedObserver; + for (InjectionPoint ip : ownerBean.getInjectionPoints()) + { + Set<Annotation> qualifiers = ip.getQualifiers(); + if (EventMetadata.class == ip.getType() + && qualifiers != null && ip.getQualifiers().size() == 1 + && Default.class == qualifiers.iterator().next().annotationType()) + { + throw new WebBeansConfigurationException(ip + " is not an observer parameter"); + } + } } - - return null; } private void checkObserverMethodConditions(AbstractOwbBean<?> bean, AnnotatedParameter<?> annotatedParameter) diff --git a/webbeans-impl/src/main/java/org/apache/webbeans/event/NotificationManager.java b/webbeans-impl/src/main/java/org/apache/webbeans/event/NotificationManager.java index 8c22c64..918730d 100644 --- a/webbeans-impl/src/main/java/org/apache/webbeans/event/NotificationManager.java +++ b/webbeans-impl/src/main/java/org/apache/webbeans/event/NotificationManager.java @@ -1150,24 +1150,40 @@ public class NotificationManager } /** - * Gets observer method from given annotated method. + * Gets container lifecycle event observer method from given annotated method. * @param <T> bean type info * @param annotatedMethod annotated method for observer * @param ownerBean bean instance * @return ObserverMethod */ - public <T> ObserverMethod<?> getObservableMethodForAnnotatedMethod(AnnotatedMethod<?> annotatedMethod, AnnotatedParameter<?> annotatedParameter, AbstractOwbBean<T> ownerBean) + public <T> ObserverMethod<?> getContainerLifecycleEventObservableMethodForAnnotatedMethod( + AnnotatedMethod<?> annotatedMethod, AnnotatedParameter<?> annotatedParameter, AbstractOwbBean<T> ownerBean) { Asserts.assertNotNull(annotatedParameter, "annotatedParameter"); ObserverMethodImpl<T> observer = null; - // Observer creation from annotated method if (isContainerEvent(annotatedParameter)) { observer = new ContainerEventObserverMethodImpl(ownerBean, annotatedMethod, annotatedParameter); addObserver(observer); } - else + return observer; + } + + /** + * Gets observer method from given annotated method. + * @param <T> bean type info + * @param annotatedMethod annotated method for observer + * @param ownerBean bean instance + * @return ObserverMethod + */ + public <T> ObserverMethod<?> getObservableMethodForAnnotatedMethod(AnnotatedMethod<?> annotatedMethod, AnnotatedParameter<?> annotatedParameter, AbstractOwbBean<T> ownerBean) + { + Asserts.assertNotNull(annotatedParameter, "annotatedParameter"); + + ObserverMethodImpl<T> observer = null; + + if (!isContainerEvent(annotatedParameter)) { observer = new ObserverMethodImpl(ownerBean, annotatedMethod, annotatedParameter); @@ -1178,7 +1194,7 @@ public class NotificationManager webBeansContext.getWebBeansUtil().inspectDefinitionErrorStack("There are errors that are added by ProcessObserverMethod event observers for " + "observer methods. Look at logs for further details"); - + if (!event.isVetoed()) { //Adds this observer @@ -1190,7 +1206,7 @@ public class NotificationManager } event.setStarted(); } - + return observer; } diff --git a/webbeans-impl/src/main/java/org/apache/webbeans/portable/events/ExtensionLoader.java b/webbeans-impl/src/main/java/org/apache/webbeans/portable/events/ExtensionLoader.java index 8a2459c..5df35b7 100644 --- a/webbeans-impl/src/main/java/org/apache/webbeans/portable/events/ExtensionLoader.java +++ b/webbeans-impl/src/main/java/org/apache/webbeans/portable/events/ExtensionLoader.java @@ -24,13 +24,16 @@ import java.io.File; import java.io.IOException; import java.net.URL; import java.util.Enumeration; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import javax.enterprise.inject.spi.AnnotatedType; import javax.enterprise.inject.spi.DefinitionException; import javax.enterprise.inject.spi.DeploymentException; import javax.enterprise.inject.spi.Extension; @@ -43,7 +46,6 @@ import org.apache.webbeans.config.WebBeansContext; import org.apache.webbeans.container.BeanManagerImpl; import org.apache.webbeans.exception.WebBeansException; import org.apache.webbeans.logger.WebBeansLoggerFacade; -import org.apache.webbeans.util.Asserts; import org.apache.webbeans.util.ExceptionUtil; import org.apache.webbeans.util.WebBeansUtil; import org.apache.xbean.finder.archive.FileArchive; @@ -95,43 +97,8 @@ public class ExtensionLoader */ public void loadExtensionServices(ClassLoader classLoader) { - Set<String> ignoredExtensions = webBeansContext.getOpenWebBeansConfiguration().getIgnoredExtensions(); - if (!ignoredExtensions.isEmpty()) - { - WebBeansLoggerFacade.getLogger(ExtensionLoader.class) - .info("Ignoring the following CDI Extensions. " + - "See " + OpenWebBeansConfiguration.IGNORED_EXTENSIONS + - " " + ignoredExtensions.toString()); - } - List<Extension> loader = webBeansContext.getLoaderService().load(Extension.class, classLoader); - for (Extension extension : loader) - { - if (ignoredExtensions.contains(extension.getClass().getName())) - { - WebBeansLoggerFacade.getLogger(ExtensionLoader.class) - .info("Skipping CDI Extension due to exclusion: " + extension.getClass().getName()); - continue; - } - - if (!extensionClasses.contains(extension.getClass())) - { - extensionClasses.add(extension.getClass()); - try - { - addExtension(extension); - } - catch (Exception e) - { - if (e instanceof DefinitionException || e instanceof DeploymentException) - { - ExceptionUtil.throwAsRuntimeException(e); - } - - throw new WebBeansException("Error occurred while reading Extension service list", e); - } - } - } + addExtensions(loader); if (!webBeansContext.getOpenWebBeansConfiguration().getScanExtensionJars()) { @@ -198,26 +165,79 @@ public class ExtensionLoader return (T) extensions.get(extensionClass); } + /** + * Add the CDI Extensions to our internal list. + * @param extensions Extensions to add + */ + public void addExtensions(List<Extension> extensions) + { + Set<String> ignoredExtensions = webBeansContext.getOpenWebBeansConfiguration().getIgnoredExtensions(); + if (!ignoredExtensions.isEmpty()) + { + WebBeansLoggerFacade.getLogger(ExtensionLoader.class) + .info("Ignoring the following CDI Extensions. " + + "See " + OpenWebBeansConfiguration.IGNORED_EXTENSIONS + + " " + ignoredExtensions.toString()); + } + + Map<ExtensionBean<Extension>, AnnotatedType> extensionBeans = new HashMap<>(extensions.size()); + for (Extension extension : extensions) + { + if (ignoredExtensions.contains(extension.getClass().getName())) + { + WebBeansLoggerFacade.getLogger(ExtensionLoader.class) + .info("Skipping CDI Extension due to exclusion: " + extension.getClass().getName()); + continue; + } + + if (!extensionClasses.contains(extension.getClass())) + { + extensionClasses.add(extension.getClass()); + try + { + final ExtensionBeanBuilder<Extension> extensionBeanBuilder = + new ExtensionBeanBuilder<>(webBeansContext, extension); + + ExtensionBean<Extension> bean = createExtensionBean(extensionBeanBuilder); + extensionBeans.put(bean, extensionBeanBuilder.getAnnotatedType()); + // since an extension can fire a ProcessInjectionPoint event when observing something else than a lifecycle event + // and at the same time observe it, we must ensure to build the observers only once the bean is available + new ObserverMethodsBuilder<>(webBeansContext, extensionBeanBuilder.getAnnotatedType()) + .defineContainerLifecycleEventObserverMethods(bean); + } + catch (Exception e) + { + if (e instanceof DefinitionException || e instanceof DeploymentException) + { + ExceptionUtil.throwAsRuntimeException(e); + } + + throw new WebBeansException("Error occurred while reading Extension service list", e); + } + } + } + // now register observers for non container lifecycle events + for (Entry<ExtensionBean<Extension>, AnnotatedType> extensionEntry : extensionBeans.entrySet()) + { + new ObserverMethodsBuilder<>(webBeansContext, extensionEntry.getValue()) + .defineObserverMethods(extensionEntry.getKey()); + } + } /** * Add a CDI Extension to our internal list. * @param ext Extension to add */ - public void addExtension(final Extension ext) + public ExtensionBean<Extension> createExtensionBean(ExtensionBeanBuilder<Extension> extensionBeanBuilder) { - final Class<Extension> extensionClass = (Class<Extension>) ext.getClass(); - Asserts.nullCheckForClass(extensionClass); - extensions.put(extensionClass, ext); + Extension extension = extensionBeanBuilder.getExtension(); + final Class<Extension> extensionClass = (Class<Extension>) extension.getClass(); + extensions.put(extensionClass, extension); - final ExtensionBeanBuilder<Extension> extensionBeanBuilder = - new ExtensionBeanBuilder<>(webBeansContext, extensionClass); - final ExtensionBean<Extension> bean = extensionBeanBuilder.getBean(); + final ExtensionBean<Extension> bean = extensionBeanBuilder.buildBean(); manager.addBean(bean); - // since an extension can fire a ProcessInjectionPoint event when observing something else than a lifecycle event - // and at the same time observe it, we must ensure to build the observers only once the bean is available - new ObserverMethodsBuilder<>(webBeansContext, extensionBeanBuilder.getAnnotatedType()) - .defineObserverMethods(bean); + return bean; } /** diff --git a/webbeans-impl/src/test/java/org/apache/webbeans/test/AbstractUnitTest.java b/webbeans-impl/src/test/java/org/apache/webbeans/test/AbstractUnitTest.java index 3323a6f..47d8be6 100644 --- a/webbeans-impl/src/test/java/org/apache/webbeans/test/AbstractUnitTest.java +++ b/webbeans-impl/src/test/java/org/apache/webbeans/test/AbstractUnitTest.java @@ -182,10 +182,7 @@ public abstract class AbstractUnitTest }; webBeansContext = WebBeansContext.getInstance(); - for (Extension ext : extensions) - { - webBeansContext.getExtensionLoader().addExtension(ext); - } + webBeansContext.getExtensionLoader().addExtensions(extensions); for (Class interceptor : interceptors) { diff --git a/webbeans-impl/src/test/java/org/apache/webbeans/test/portable/events/ProcessObserverMethodTest.java b/webbeans-impl/src/test/java/org/apache/webbeans/test/portable/events/ProcessObserverMethodTest.java new file mode 100644 index 0000000..90ca557 --- /dev/null +++ b/webbeans-impl/src/test/java/org/apache/webbeans/test/portable/events/ProcessObserverMethodTest.java @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.webbeans.test.portable.events; + +import java.util.ArrayList; +import java.util.Collection; + +import javax.enterprise.context.spi.Context; +import javax.enterprise.event.Observes; + +import org.apache.webbeans.test.AbstractUnitTest; +import org.apache.webbeans.test.portable.events.extensions.ProcessObserverMethodExtension; +import org.junit.Assert; +import org.junit.Test; + +public class ProcessObserverMethodTest extends AbstractUnitTest +{ + + @Test + public void testProcessObserverMethodIsInvoked() + { + Collection<String> beanXmls = new ArrayList<String>(); + + Collection<Class<?>> beanClasses = new ArrayList<Class<?>>(); + beanClasses.add(MyObserver.class); + + addExtension(new ProcessObserverMethodExtension.BrokenExtension()); + addExtension(new ProcessObserverMethodExtension()); + + startContainer(beanClasses, beanXmls); + + Assert.assertTrue(ProcessObserverMethodExtension.processObserverMethodInvoked); + + shutDownContainer(); + } + + public static class MyObserver { + public void observe(@Observes Context context) { + // just any observer + } + } +} diff --git a/webbeans-impl/src/test/java/org/apache/webbeans/test/portable/events/extensions/ProcessObserverMethodExtension.java b/webbeans-impl/src/test/java/org/apache/webbeans/test/portable/events/extensions/ProcessObserverMethodExtension.java new file mode 100644 index 0000000..c0c7eb3 --- /dev/null +++ b/webbeans-impl/src/test/java/org/apache/webbeans/test/portable/events/extensions/ProcessObserverMethodExtension.java @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.webbeans.test.portable.events.extensions; + +import javax.enterprise.context.spi.Context; +import javax.enterprise.event.Observes; +import javax.enterprise.inject.spi.Extension; +import javax.enterprise.inject.spi.ProcessObserverMethod; + +public class ProcessObserverMethodExtension implements Extension +{ + public static boolean processObserverMethodInvoked = false; + + public void processObserverMethod(@Observes ProcessObserverMethod<?, ?> event) + { + processObserverMethodInvoked = true; + } + + public static class BrokenExtension implements Extension + { + public void listenToNonLifecycleEvent(@Observes Context context) + { + // do nothing + } + } +}
