Author: marius.bogoevici Date: 2009-10-16 16:22:51 -0400 (Fri, 16 Oct 2009) New Revision: 4159 Modified: core/trunk/impl/src/main/java/org/jboss/weld/BeanManagerImpl.java core/trunk/impl/src/main/java/org/jboss/weld/Validator.java core/trunk/impl/src/main/java/org/jboss/weld/bean/AbstractClassBean.java core/trunk/impl/src/main/java/org/jboss/weld/bean/ManagedBean.java core/trunk/impl/src/main/java/org/jboss/weld/bean/SessionBean.java core/trunk/impl/src/main/java/org/jboss/weld/util/Beans.java Log: WELD-14, fixing the way in which the validation is done (after bean initialization) Modified: core/trunk/impl/src/main/java/org/jboss/weld/BeanManagerImpl.java =================================================================== --- core/trunk/impl/src/main/java/org/jboss/weld/BeanManagerImpl.java 2009-10-16 20:20:59 UTC (rev 4158) +++ core/trunk/impl/src/main/java/org/jboss/weld/BeanManagerImpl.java 2009-10-16 20:22:51 UTC (rev 4159) @@ -589,7 +589,7 @@ public void addInterceptor(InterceptorImpl<?> bean) { interceptors.add(bean); - //TODO decide if interceptor is passivationCapable + getServices().get(ContextualStore.class).putIfAbsent(bean); interceptorResolver.clear(); } @@ -1401,12 +1401,12 @@ this.currentInjectionPoint.remove(); } - public InterceptorRegistry<Class<?>, SerializableContextual<Interceptor<?>, ?>> getBoundInterceptorsRegistry() + public InterceptorRegistry<Class<?>, SerializableContextual<Interceptor<?>, ?>> getCdiInterceptorsRegistry() { return boundInterceptorsRegistry; } - public InterceptorRegistry<Class<?>, Class<?>> getDeclaredInterceptorsRegistry() + public InterceptorRegistry<Class<?>, Class<?>> getClassDeclaredInterceptorsRegistry() { return declaredInterceptorsRegistry; } Modified: core/trunk/impl/src/main/java/org/jboss/weld/Validator.java =================================================================== --- core/trunk/impl/src/main/java/org/jboss/weld/Validator.java 2009-10-16 20:20:59 UTC (rev 4158) +++ core/trunk/impl/src/main/java/org/jboss/weld/Validator.java 2009-10-16 20:22:51 UTC (rev 4159) @@ -38,6 +38,8 @@ import javax.enterprise.inject.spi.Bean; import javax.enterprise.inject.spi.Decorator; import javax.enterprise.inject.spi.InjectionPoint; +import javax.enterprise.inject.spi.Interceptor; +import javax.enterprise.inject.spi.InjectionTarget; import org.jboss.weld.bean.AbstractClassBean; import org.jboss.weld.bean.AbstractProducerBean; @@ -46,6 +48,7 @@ import org.jboss.weld.bean.NewManagedBean; import org.jboss.weld.bean.NewSessionBean; import org.jboss.weld.bean.RIBean; +import org.jboss.weld.bean.ManagedBean; import org.jboss.weld.bootstrap.BeanDeployerEnvironment; import org.jboss.weld.bootstrap.api.Service; import org.jboss.weld.introspector.WeldAnnotated; @@ -54,6 +57,8 @@ import org.jboss.weld.util.Beans; import org.jboss.weld.util.Proxies; import org.jboss.weld.util.Reflections; +import org.jboss.weld.context.SerializableContextual; +import org.jboss.interceptor.model.InterceptionModel; import com.google.common.base.Supplier; import com.google.common.collect.Multimap; @@ -109,24 +114,89 @@ AbstractClassBean<?> classBean = (AbstractClassBean<?>) bean; if (classBean.hasDecorators()) { - for (Decorator<?> decorator : classBean.getDecorators()) + validateDecorators(beanManager, classBean); + } + // validate CDI-defined interceptors + if (classBean.hasCdiBoundInterceptors()) + { + validateCdiBoundInterceptors(beanManager, classBean); + } + // validate EJB-defined interceptors + if (classBean instanceof ManagedBean && ((ManagedBean)classBean).hasDirectlyDefinedInterceptors()) + { + validateDirectlyDefinedInterceptorClasses(beanManager, classBean); + } + } + } + } + + private void validateDirectlyDefinedInterceptorClasses(BeanManagerImpl beanManager, AbstractClassBean<?> classBean) + { + InterceptionModel<Class<?>, Class<?>> ejbInterceptorModel = beanManager.getClassDeclaredInterceptorsRegistry().getInterceptionModel(((AbstractClassBean<?>) classBean).getType()); + if (ejbInterceptorModel != null) + { + Class<?>[] classDeclaredInterceptors = ejbInterceptorModel.getAllInterceptors().toArray(new Class<?>[0]); + if (classDeclaredInterceptors != null) + { + for (Class<?> interceptorClass: classDeclaredInterceptors) + { + if (!Reflections.isSerializable(interceptorClass)) { - if (!Reflections.isSerializable(decorator.getBeanClass())) + throw new DeploymentException("The bean " + this + " declared a passivating scope, " + + "but has a non-serializable interceptor class: " + interceptorClass.getName()); + } + InjectionTarget<Object> injectionTarget = (InjectionTarget<Object>) beanManager.createInjectionTarget(beanManager.createAnnotatedType(interceptorClass)); + for (InjectionPoint injectionPoint: injectionTarget.getInjectionPoints()) + { + Bean<?> resolvedBean = beanManager.resolve(beanManager.getInjectableBeans(injectionPoint)); + validateInjectionPointPassivationCapable(injectionPoint, resolvedBean, beanManager); + } + } + } + } + } + + private void validateCdiBoundInterceptors(BeanManagerImpl beanManager, AbstractClassBean<?> classBean) + { + InterceptionModel<Class<?>, SerializableContextual<Interceptor<?>, ?>> cdiInterceptorModel = beanManager.getCdiInterceptorsRegistry().getInterceptionModel(((AbstractClassBean<?>) classBean).getType()); + if (cdiInterceptorModel != null) + { + Collection<SerializableContextual<Interceptor<?>, ?>> interceptors = cdiInterceptorModel.getAllInterceptors(); + if (interceptors.size() > 0) + { + for (SerializableContextual<Interceptor<?>, ?> serializableContextual : interceptors) { - throw new UnserializableDependencyException("The bean " + bean + " declares a passivating scope but has non-serializable decorator: " + decorator); + if (!Reflections.isSerializable(serializableContextual.get().getBeanClass())) + { + throw new DeploymentException("The bean " + this + " declared a passivating scope " + + "but has a non-serializable interceptor: " + serializableContextual.get()); + } + for (InjectionPoint injectionPoint: serializableContextual.get().getInjectionPoints()) + { + Bean<?> resolvedBean = beanManager.resolve(beanManager.getInjectableBeans(injectionPoint)); + validateInjectionPointPassivationCapable(injectionPoint, resolvedBean, beanManager); + } } - for (InjectionPoint ij : decorator.getInjectionPoints()) - { - Bean<?> resolvedBean = beanManager.resolve(beanManager.getInjectableBeans(ij)); - Beans.validateInjectionPointPassivationCapable(ij, resolvedBean, beanManager); - } } } - + } + + private void validateDecorators(BeanManagerImpl beanManager, AbstractClassBean<?> classBean) + { + for (Decorator<?> decorator : classBean.getDecorators()) + { + if (!Reflections.isSerializable(decorator.getBeanClass())) + { + throw new UnserializableDependencyException("The bean " + classBean + " declares a passivating scope but has non-serializable decorator: " + decorator); } + for (InjectionPoint ij : decorator.getInjectionPoints()) + { + Bean<?> resolvedBean = beanManager.resolve(beanManager.getInjectableBeans(ij)); + validateInjectionPointPassivationCapable(ij, resolvedBean, beanManager); + } } } - + /** * Validate an injection point * @@ -176,10 +246,22 @@ } if (ij.getBean() != null && Beans.isPassivatingScope(ij.getBean(), beanManager) && (!ij.isTransient()) && !Beans.isPassivationCapableBean(resolvedBean)) { - Beans.validateInjectionPointPassivationCapable(ij, resolvedBean, beanManager); + validateInjectionPointPassivationCapable(ij, resolvedBean, beanManager); } } + public void validateInjectionPointPassivationCapable(InjectionPoint ij, Bean<?> resolvedBean, BeanManagerImpl beanManager) + { + if (!ij.isTransient() && !Beans.isPassivationCapableBean(resolvedBean)) + { + if (resolvedBean.getScope().equals(Dependent.class) && resolvedBean instanceof AbstractProducerBean<?, ?,?>) + { + throw new IllegalProductException("The bean " + ij.getBean() + " declares a passivating scope but the producer returned a non-serializable bean for injection: " + resolvedBean); + } + throw new UnserializableDependencyException("The bean " + ij.getBean() + " declares a passivating scope but has non-serializable dependency: " + resolvedBean); + } + } + public void validateDeployment(BeanManagerImpl manager, BeanDeployerEnvironment environment) { validateBeans(manager.getDecorators(), new ArrayList<RIBean<?>>(), manager); @@ -264,7 +346,8 @@ } } } - + + private void validateEnabledPolicies(BeanManagerImpl beanManager) { List<Class<?>> seenPolicies = new ArrayList<Class<?>>(); @@ -327,6 +410,7 @@ } + public void cleanup() {} } Modified: core/trunk/impl/src/main/java/org/jboss/weld/bean/AbstractClassBean.java =================================================================== --- core/trunk/impl/src/main/java/org/jboss/weld/bean/AbstractClassBean.java 2009-10-16 20:20:59 UTC (rev 4158) +++ core/trunk/impl/src/main/java/org/jboss/weld/bean/AbstractClassBean.java 2009-10-16 20:22:51 UTC (rev 4159) @@ -24,7 +24,6 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Set; -import java.util.Collection; import javassist.util.proxy.ProxyFactory; import javassist.util.proxy.ProxyObject; @@ -38,7 +37,6 @@ import javax.enterprise.inject.spi.InjectionTarget; import javax.enterprise.inject.spi.InterceptionType; import javax.enterprise.inject.spi.Interceptor; -import javax.enterprise.inject.spi.Bean; import javax.inject.Scope; import org.jboss.interceptor.model.InterceptionModelBuilder; @@ -59,7 +57,6 @@ import org.jboss.weld.util.Beans; import org.jboss.weld.util.Proxies; import org.jboss.weld.util.Strings; -import org.jboss.weld.util.Reflections; /** * An abstract bean representation common for class-based beans @@ -413,7 +410,7 @@ protected void initInterceptors() { - if (manager.getBoundInterceptorsRegistry().getInterceptionModel(getType()) == null) + if (manager.getCdiInterceptorsRegistry().getInterceptionModel(getType()) == null) { InterceptionModelBuilder<Class<?>, SerializableContextual<Interceptor<?>, ?>> builder = InterceptionModelBuilder.newBuilderFor(getType(), (Class) SerializableContextual.class); @@ -430,20 +427,15 @@ Annotation[] classBindingAnnotationsArray = classBindingAnnotations.toArray(new Annotation[0]); List<Interceptor<?>> resolvedPostConstructInterceptors = manager.resolveInterceptors(InterceptionType.POST_CONSTRUCT, classBindingAnnotationsArray); - validateSerializableInterceptors(resolvedPostConstructInterceptors); builder.interceptPostConstruct().with(toSerializableContextualArray(resolvedPostConstructInterceptors)); List<Interceptor<?>> resolvedPreDestroyInterceptors = manager.resolveInterceptors(InterceptionType.PRE_DESTROY, classBindingAnnotationsArray); - validateSerializableInterceptors(resolvedPreDestroyInterceptors); builder.interceptPreDestroy().with(toSerializableContextualArray(resolvedPreDestroyInterceptors)); - List<Interceptor<?>> resolvedPrePassivateInterceptors = manager.resolveInterceptors(InterceptionType.PRE_PASSIVATE, classBindingAnnotationsArray); - validateSerializableInterceptors(resolvedPrePassivateInterceptors); builder.interceptPrePassivate().with(toSerializableContextualArray(resolvedPrePassivateInterceptors)); List<Interceptor<?>> resolvedPostActivateInterceptors = manager.resolveInterceptors(InterceptionType.POST_ACTIVATE, classBindingAnnotationsArray); - validateSerializableInterceptors(resolvedPostActivateInterceptors); builder.interceptPostActivate().with(toSerializableContextualArray(resolvedPostActivateInterceptors)); } @@ -457,33 +449,12 @@ if (Beans.findInterceptorBindingConflicts(manager, classBindingAnnotations)) throw new DeploymentException("Conflicting interceptor bindings found on " + getType() + "." + method.getName() + "()"); List<Interceptor<?>> methodBoundInterceptors = manager.resolveInterceptors(InterceptionType.AROUND_INVOKE, methodBindingAnnotations.toArray(new Annotation[]{})); - validateSerializableInterceptors(methodBoundInterceptors); builder.interceptAroundInvoke(((AnnotatedMethod) method).getJavaMember()).with(toSerializableContextualArray(methodBoundInterceptors)); } } - manager.getBoundInterceptorsRegistry().registerInterceptionModel(getType(), builder.build()); + manager.getCdiInterceptorsRegistry().registerInterceptionModel(getType(), builder.build()); } } - - private void validateSerializableInterceptors(Collection<Interceptor<?>> interceptors) - { - if (Beans.isPassivationCapableBean(this)) - { - for (Interceptor<?> interceptor: interceptors) - { - if (!Reflections.isSerializable(interceptor.getBeanClass())) - { - throw new DeploymentException("The bean " + this + " declared a passivating scope, " + - "but has a non-serializable interceptor: " + interceptor); - } - for (InjectionPoint injectionPoint: interceptor.getInjectionPoints()) - { - Bean<?> resolvedBean = manager.resolve(manager.getInjectableBeans(injectionPoint)); - Beans.validateInjectionPointPassivationCapable(injectionPoint, resolvedBean, manager); - } - } - } - } public void setInjectionTarget(InjectionTarget<T> injectionTarget) { @@ -544,4 +515,13 @@ return serializableContextuals.toArray(new SerializableContextual[]{}); } + public boolean hasCdiBoundInterceptors() + { + if (manager.getCdiInterceptorsRegistry().getInterceptionModel(getType()) != null) + return manager.getCdiInterceptorsRegistry().getInterceptionModel(getType()).getAllInterceptors().size() > 0; + else + return false; + } + + } Modified: core/trunk/impl/src/main/java/org/jboss/weld/bean/ManagedBean.java =================================================================== --- core/trunk/impl/src/main/java/org/jboss/weld/bean/ManagedBean.java 2009-10-16 20:20:59 UTC (rev 4158) +++ core/trunk/impl/src/main/java/org/jboss/weld/bean/ManagedBean.java 2009-10-16 20:22:51 UTC (rev 4159) @@ -28,7 +28,6 @@ import javax.enterprise.inject.spi.Decorator; import javax.enterprise.inject.spi.InjectionPoint; import javax.enterprise.inject.spi.InjectionTarget; -import javax.enterprise.inject.spi.Bean; import org.jboss.interceptor.model.InterceptionModelBuilder; import org.jboss.interceptor.model.InterceptorClassMetadataImpl; @@ -114,7 +113,7 @@ { T instance = getInjectionTarget().produce(creationalContext); getInjectionTarget().inject(instance, creationalContext); - if (isInterceptionCandidate() && (hasBoundInterceptors() || hasDeclaredInterceptors())) + if (isInterceptionCandidate() && (hasCdiBoundInterceptors() || hasDirectlyDefinedInterceptors())) { InterceptionUtils.executePostConstruct(instance); } @@ -148,7 +147,7 @@ { try { - if (!isInterceptionCandidate() || !hasBoundInterceptors()) + if (!isInterceptionCandidate() || !hasCdiBoundInterceptors()) { getInjectionTarget().preDestroy(instance); } @@ -180,7 +179,7 @@ initEEInjectionPoints(); if (isInterceptionCandidate()) { - initDeclaredInterceptors(); + initClassInterceptors(); } setInjectionTarget(new InjectionTarget<T>() { @@ -237,7 +236,7 @@ { instance = applyDecorators(instance, ctx, originalInjectionPoint); } - if (isInterceptionCandidate() && (hasBoundInterceptors() || hasDeclaredInterceptors())) + if (isInterceptionCandidate() && (hasCdiBoundInterceptors() || hasDirectlyDefinedInterceptors())) { instance = applyInterceptors(instance, ctx); } @@ -419,36 +418,28 @@ return !Beans.isInterceptor(getAnnotatedItem()) && !Beans.isDecorator(getAnnotatedItem()); } - public boolean hasBoundInterceptors() + public boolean hasDirectlyDefinedInterceptors() { - if (manager.getBoundInterceptorsRegistry().getInterceptionModel(getType()) != null) - return manager.getBoundInterceptorsRegistry().getInterceptionModel(getType()).getAllInterceptors().size() > 0; + if (manager.getClassDeclaredInterceptorsRegistry().getInterceptionModel(getType()) != null) + return manager.getClassDeclaredInterceptorsRegistry().getInterceptionModel(getType()).getAllInterceptors().size() > 0; else return false; } - private boolean hasDeclaredInterceptors() - { - if (manager.getDeclaredInterceptorsRegistry().getInterceptionModel(getType()) != null) - return manager.getDeclaredInterceptorsRegistry().getInterceptionModel(getType()).getAllInterceptors().size() > 0; - else - return false; - } - protected T applyInterceptors(T instance, final CreationalContext<T> creationalContext) { try { List<InterceptorRegistry<Class<?>, ?>> interceptionRegistries = new ArrayList<InterceptorRegistry<Class<?>,?>>(); List<InterceptionHandlerFactory<?>> interceptionHandlerFactories = new ArrayList<InterceptionHandlerFactory<?>>(); - if (hasDeclaredInterceptors()) + if (hasDirectlyDefinedInterceptors()) { - interceptionRegistries.add(manager.getDeclaredInterceptorsRegistry()); + interceptionRegistries.add(manager.getClassDeclaredInterceptorsRegistry()); interceptionHandlerFactories.add(new ClassInterceptionHandlerFactory(creationalContext, getManager())); } - if (hasBoundInterceptors()) + if (hasCdiBoundInterceptors()) { - interceptionRegistries.add(manager.getBoundInterceptorsRegistry()); + interceptionRegistries.add(manager.getCdiInterceptorsRegistry()); interceptionHandlerFactories.add(new CdiInterceptorHandlerFactory(creationalContext, manager)); } if (interceptionRegistries.size() > 0) @@ -461,9 +452,9 @@ return instance; } - protected void initDeclaredInterceptors() + protected void initClassInterceptors() { - if (manager.getDeclaredInterceptorsRegistry().getInterceptionModel(getType()) == null && InterceptionUtils.supportsEjb3InterceptorDeclaration()) + if (manager.getClassDeclaredInterceptorsRegistry().getInterceptionModel(getType()) == null && InterceptionUtils.supportsEjb3InterceptorDeclaration()) { InterceptionModelBuilder<Class<?>, Class<?>> builder = InterceptionModelBuilder.newBuilderFor(getType(), (Class) Class.class); @@ -474,9 +465,6 @@ classDeclaredInterceptors = Reflections.extractValues(interceptorsAnnotation); } - validatePassivatingInterceptors(classDeclaredInterceptors); - - if (classDeclaredInterceptors != null) { builder.interceptPostConstruct().with(classDeclaredInterceptors); @@ -494,7 +482,6 @@ { methodDeclaredInterceptors = Reflections.extractValues(method.getAnnotation(InterceptionUtils.getInterceptorsAnnotationClass())); } - validatePassivatingInterceptors(methodDeclaredInterceptors); if (!excludeClassInterceptors && classDeclaredInterceptors != null) { builder.interceptAroundInvoke(((AnnotatedMethod) method).getJavaMember()).with(classDeclaredInterceptors); @@ -506,29 +493,8 @@ } InterceptionModel<Class<?>, Class<?>> interceptionModel = builder.build(); if (interceptionModel.getAllInterceptors().size() > 0 || new InterceptorClassMetadataImpl(getType()).isInterceptor()) - manager.getDeclaredInterceptorsRegistry().registerInterceptionModel(getType(), builder.build()); + manager.getClassDeclaredInterceptorsRegistry().registerInterceptionModel(getType(), builder.build()); } } - private void validatePassivatingInterceptors(Class<?>[] classDeclaredInterceptors) - { - if (classDeclaredInterceptors != null && Beans.isPassivationCapableBean(this)) - { - for (Class<?> interceptorClass: classDeclaredInterceptors) - { - if (!Reflections.isSerializable(interceptorClass)) - { - throw new DeploymentException("The bean " + this + " declared a passivating scope, " + - "but has a non-serializable interceptor class: " + interceptorClass.getName()); - } - InjectionTarget<Object> injectionTarget = (InjectionTarget<Object>) manager.createInjectionTarget(manager.createAnnotatedType(interceptorClass)); - for (InjectionPoint injectionPoint: injectionTarget.getInjectionPoints()) - { - Bean<?> resolvedBean = manager.resolve(manager.getInjectableBeans(injectionPoint)); - Beans.validateInjectionPointPassivationCapable(injectionPoint, resolvedBean, manager); - } - } - } - } - } Modified: core/trunk/impl/src/main/java/org/jboss/weld/bean/SessionBean.java =================================================================== --- core/trunk/impl/src/main/java/org/jboss/weld/bean/SessionBean.java 2009-10-16 20:20:59 UTC (rev 4158) +++ core/trunk/impl/src/main/java/org/jboss/weld/bean/SessionBean.java 2009-10-16 20:22:51 UTC (rev 4159) @@ -431,7 +431,7 @@ private void registerInterceptors() { - InterceptionModel<Class<?>, SerializableContextual<javax.enterprise.inject.spi.Interceptor<?>, ?>> model = manager.getBoundInterceptorsRegistry().getInterceptionModel(ejbDescriptor.getBeanClass()); + InterceptionModel<Class<?>, SerializableContextual<javax.enterprise.inject.spi.Interceptor<?>, ?>> model = manager.getCdiInterceptorsRegistry().getInterceptionModel(ejbDescriptor.getBeanClass()); if (model != null) getManager().getServices().get(EjbServices.class).registerInterceptors(getEjbDescriptor(), new InterceptorBindingsAdapter(model)); } Modified: core/trunk/impl/src/main/java/org/jboss/weld/util/Beans.java =================================================================== --- core/trunk/impl/src/main/java/org/jboss/weld/util/Beans.java 2009-10-16 20:20:59 UTC (rev 4158) +++ core/trunk/impl/src/main/java/org/jboss/weld/util/Beans.java 2009-10-16 20:22:51 UTC (rev 4159) @@ -34,21 +34,17 @@ import javax.decorator.Decorator; import javax.enterprise.context.spi.Contextual; import javax.enterprise.context.spi.CreationalContext; -import javax.enterprise.context.Dependent; import javax.enterprise.event.Observes; import javax.enterprise.inject.CreationException; import javax.enterprise.inject.Disposes; import javax.enterprise.inject.Produces; -import javax.enterprise.inject.IllegalProductException; import javax.enterprise.inject.spi.Bean; -import javax.enterprise.inject.spi.InjectionPoint; import javax.inject.Inject; import org.jboss.interceptor.model.InterceptionType; import org.jboss.interceptor.model.InterceptionTypeRegistry; import org.jboss.weld.BeanManagerImpl; import org.jboss.weld.DefinitionException; -import org.jboss.weld.UnserializableDependencyException; import org.jboss.weld.bean.AbstractProducerBean; import org.jboss.weld.bean.RIBean; import org.jboss.weld.bean.SessionBean; @@ -132,18 +128,6 @@ } } - public static void validateInjectionPointPassivationCapable(InjectionPoint ij, Bean<?> resolvedBean, BeanManagerImpl beanManager) - { - if (!ij.isTransient() && !Beans.isPassivationCapableBean(resolvedBean)) - { - if (resolvedBean.getScope().equals(Dependent.class) && resolvedBean instanceof AbstractProducerBean<?, ?, ?>) - { - throw new IllegalProductException("The bean " + ij.getBean() + " declares a passivating scope but the producer returned a non-serializable bean for injection: " + resolvedBean); - } - throw new UnserializableDependencyException("The bean " + ij.getBean() + " declares a passivating scope but has non-serializable dependency: " + resolvedBean); - } - } - /** * Indicates if a bean is proxyable *