Skip to content

Commit

Permalink
WELD-2271 Validate notify() methods of a custom observer method
Browse files Browse the repository at this point in the history
  • Loading branch information
mkouba committed Dec 13, 2016
1 parent f234184 commit 99afc94
Show file tree
Hide file tree
Showing 13 changed files with 364 additions and 14 deletions.
Expand Up @@ -258,7 +258,6 @@ private void finish() {
}
for (ObserverMethodConfigurator<?> configurator : additionalObserverConfigurators) {
ObserverMethod<?> observer = new ObserverMethodBuilderImpl<>(cast(configurator)).build();
validateObserverMethod(observer, getBeanManager(), null);
processAdditionalObserver(observer);
}
} catch (Exception e) {
Expand All @@ -267,6 +266,7 @@ private void finish() {
}

private void processAdditionalObserver(ObserverMethod<?> observer) {
validateObserverMethod(observer, getBeanManager(), null);
BeanManagerImpl manager = getOrCreateBeanDeployment(observer.getBeanClass()).getBeanManager();
if (Observers.isObserverMethodEnabled(observer, manager)) {
ObserverMethod<?> processedObserver = containerLifecycleEvents.fireProcessObserverMethod(manager, observer);
Expand Down
Expand Up @@ -29,6 +29,7 @@
import javax.enterprise.inject.spi.configurator.ObserverMethodConfigurator.EventConsumer;

import org.jboss.weld.event.SyntheticObserverMethod;
import org.jboss.weld.logging.EventLogger;
import org.jboss.weld.util.collections.ImmutableSet;

/**
Expand Down Expand Up @@ -84,6 +85,9 @@ static class ImmutableObserverMethod<T> implements SyntheticObserverMethod<T> {
* @param configurator
*/
ImmutableObserverMethod(ObserverMethodConfiguratorImpl<T> configurator) {
if (configurator.getNotifyCallback() == null) {
throw EventLogger.LOG.notifyMethodNotImplemented(configurator);
}
this.beanClass = configurator.getBeanClass();
this.observedType = configurator.getObservedType();
this.observedQualifiers = ImmutableSet.copyOf(configurator.getObservedQualifiers());
Expand Down
Expand Up @@ -35,7 +35,7 @@
import javax.enterprise.inject.spi.ObserverMethod;
import javax.enterprise.inject.spi.configurator.ObserverMethodConfigurator;

import org.jboss.weld.exceptions.IllegalArgumentException;
import org.jboss.weld.logging.EventLogger;
import org.jboss.weld.util.reflection.Formats;

/**
Expand Down Expand Up @@ -70,6 +70,7 @@ public ObserverMethodConfiguratorImpl() {
public ObserverMethodConfiguratorImpl(ObserverMethod<T> observerMethod) {
this();
read(observerMethod);
notifyWith(e -> observerMethod.notify(e));
}

@Override
Expand Down Expand Up @@ -126,7 +127,6 @@ public ObserverMethodConfigurator<T> read(final ObserverMethod<T> observerMethod
transactionPhase(observerMethod.getTransactionPhase());
priority(observerMethod.getPriority());
async(observerMethod.isAsync());
notifyWith(e -> observerMethod.notify(e));
return this;
}

Expand Down Expand Up @@ -238,9 +238,7 @@ EventConsumer<T> getNotifyCallback() {

private <P> void checkEventParams(Set<P> eventParams, Method method) {
if (eventParams.size() != 1) {
// TODO Move to EventLogger in case of the read methods remain in the spec
throw new IllegalArgumentException(
"None or multiple event parameters declared on: " + method + "\n\tat " + Formats.formatAsStackTraceElement(method) + "\n StackTrace:");
EventLogger.LOG.noneOrMultipleEventParametersDeclared(method, Formats.formatAsStackTraceElement(method));
}
}

Expand Down
7 changes: 7 additions & 0 deletions impl/src/main/java/org/jboss/weld/logging/EventLogger.java
Expand Up @@ -27,6 +27,7 @@
import org.jboss.logging.annotations.Message.Format;
import org.jboss.logging.annotations.MessageLogger;
import org.jboss.weld.exceptions.DefinitionException;
import org.jboss.weld.exceptions.IllegalArgumentException;
import org.jboss.weld.exceptions.InvalidObjectException;

/**
Expand Down Expand Up @@ -85,4 +86,10 @@ public interface EventLogger extends WeldLogger {
@Message(id = 414, value = "Observer method for container lifecycle events cannot be asynchronous. {0}\n\tat {1}\n StackTrace:", format = Format.MESSAGE_FORMAT)
DefinitionException asyncContainerLifecycleEventObserver(ObserverMethod<?> observer, Object stackElement);

@Message(id = 415, value = "Custom implementation of observer method does not override either notify(T) or notify(EventContext<T>): {0}", format = Format.MESSAGE_FORMAT)
DefinitionException notifyMethodNotImplemented(Object observer);

@Message(id = 416, value = "None or multiple event parameters declared on: {0}\n\tat {1}\n StackTrace:", format = Format.MESSAGE_FORMAT)
IllegalArgumentException noneOrMultipleEventParametersDeclared(Object method, Object stackElement);

}
21 changes: 21 additions & 0 deletions impl/src/main/java/org/jboss/weld/util/Observers.java
Expand Up @@ -17,6 +17,8 @@
package org.jboss.weld.util;

import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.security.AccessController;
import java.util.Set;

import javax.enterprise.inject.spi.AfterBeanDiscovery;
Expand Down Expand Up @@ -46,8 +48,10 @@
import org.jboss.weld.event.ContainerLifecycleEventObserverMethod;
import org.jboss.weld.event.EventMetadataAwareObserverMethod;
import org.jboss.weld.event.ObserverMethodImpl;
import org.jboss.weld.event.SyntheticObserverMethod;
import org.jboss.weld.logging.EventLogger;
import org.jboss.weld.manager.BeanManagerImpl;
import org.jboss.weld.security.GetDeclaredMethodsAction;
import org.jboss.weld.util.collections.ImmutableSet;
import org.jboss.weld.util.reflection.Reflections;

Expand All @@ -70,6 +74,8 @@ public class Observers {
ProcessProducerField.class, ProcessObserverMethod.class)
.build();

private static final String NOTIFY_METHOD_NAME = "notify";

private Observers() {
}

Expand Down Expand Up @@ -111,6 +117,9 @@ public static void validateObserverMethod(ObserverMethod<?> observerMethod, Bean
if (originalObserverMethod != null && (!observerMethod.getBeanClass().equals(originalObserverMethod.getBeanClass()))) {
throw EventLogger.LOG.beanClassMismatch(originalObserverMethod, observerMethod);
}
if (!(observerMethod instanceof SyntheticObserverMethod) && !hasNotifyOverriden(observerMethod.getClass(), observerMethod)) {
throw EventLogger.LOG.notifyMethodNotImplemented(observerMethod);
}
}

/**
Expand All @@ -135,6 +144,18 @@ public static <T> void notify(ObserverMethod<? super T> observerMethod, T event,
observerMethod.notify(new EventContextImpl<>(event, metadata));
}

private static boolean hasNotifyOverriden(Class<?> clazz, ObserverMethod<?> observerMethod) {
if (clazz.isInterface()) {
return false;
}
for (Method method : AccessController.doPrivileged(new GetDeclaredMethodsAction(clazz))) {
if (NOTIFY_METHOD_NAME.equals(method.getName()) && method.getParameterTypes().length == 1) {
return true;
}
}
return clazz.getSuperclass() != null ? hasNotifyOverriden(clazz.getSuperclass(), observerMethod) : false;
}

static class EventContextImpl<T> implements EventContext<T> {

private final T event;
Expand Down
Expand Up @@ -461,7 +461,7 @@ public static void checkDeclaringClassLoadable(Class<?> c) {
* @return method method with the given name declared by the given class or null if no such method exists
*/
public static Method findDeclaredMethodByName(Class<?> clazz, String methodName) {
for (Method method : clazz.getDeclaredMethods()) {
for (Method method : AccessController.doPrivileged(new GetDeclaredMethodsAction(clazz))) {
if (methodName.equals(method.getName())) {
return method;
}
Expand Down
7 changes: 0 additions & 7 deletions jboss-tck-runner/src/test/tck20/tck-tests.xml
Expand Up @@ -123,13 +123,6 @@
</methods>
</class>

<!-- WELD-2271 -->
<class name="org.jboss.cdi.tck.tests.extensions.lifecycle.broken.observerMethod.ObserverMethodWithoutNotifyMethodTest">
<methods>
<exclude name=".*"/>
</methods>
</class>

<!-- WELD-2263 -->
<class name="org.jboss.cdi.tck.tests.extensions.lifecycle.processBeanAttributes.ignoreFinalMethods.IgnoreFinalMethodsTest">
<methods>
Expand Down
@@ -0,0 +1,63 @@
/*
* JBoss, Home of Professional Open Source
* Copyright 2016, Red Hat, Inc., and individual contributors
* by the @authors tag. See the copyright.txt in the distribution for a
* full listing of individual contributors.
*
* Licensed 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.jboss.weld.tests.observers.custom;

import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.util.Collections;
import java.util.Set;

import javax.enterprise.event.Observes;
import javax.enterprise.event.Reception;
import javax.enterprise.event.TransactionPhase;
import javax.enterprise.inject.spi.AfterBeanDiscovery;
import javax.enterprise.inject.spi.Extension;
import javax.enterprise.inject.spi.ObserverMethod;

public class CustomObserverExtension implements Extension {

public <T> void registerObservers(@Observes AfterBeanDiscovery event) {
event.addObserverMethod(new ObserverMethod<Long>() {

@Override
public Class<?> getBeanClass() {
return CustomObserverExtension.class;
}

@Override
public Type getObservedType() {
return Long.class;
}

@Override
public Set<Annotation> getObservedQualifiers() {
return Collections.emptySet();
}

@Override
public Reception getReception() {
return Reception.ALWAYS;
}

@Override
public TransactionPhase getTransactionPhase() {
return TransactionPhase.IN_PROGRESS;
}
});
}

}
@@ -0,0 +1,78 @@
/*
* JBoss, Home of Professional Open Source
* Copyright 2016, Red Hat, Inc., and individual contributors
* by the @authors tag. See the copyright.txt in the distribution for a
* full listing of individual contributors.
*
* Licensed 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.jboss.weld.tests.observers.custom;

import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.util.Collections;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;

import javax.enterprise.event.Observes;
import javax.enterprise.event.Reception;
import javax.enterprise.event.TransactionPhase;
import javax.enterprise.inject.spi.AfterBeanDiscovery;
import javax.enterprise.inject.spi.Extension;
import javax.enterprise.inject.spi.ObserverMethod;

public class CustomObserverSuperclassExtension implements Extension {

static final AtomicBoolean OBSERVED = new AtomicBoolean(false);

public <T> void registerObservers(@Observes AfterBeanDiscovery event) {
event.addObserverMethod(new CustomObserver());
}

static class CustomObserver extends AbstractObserver {

@Override
public Class<?> getBeanClass() {
return CustomObserverSuperclassExtension.class;
}

@Override
public Type getObservedType() {
return Long.class;
}

@Override
public Set<Annotation> getObservedQualifiers() {
return Collections.emptySet();
}

@Override
public Reception getReception() {
return Reception.ALWAYS;
}

@Override
public TransactionPhase getTransactionPhase() {
return TransactionPhase.IN_PROGRESS;
}

}

static abstract class AbstractObserver implements ObserverMethod<Long> {

@Override
public void notify(Long event) {
OBSERVED.set(true);
}

}

}
@@ -0,0 +1,53 @@
/*
* JBoss, Home of Professional Open Source
* Copyright 2016, Red Hat, Inc., and individual contributors
* by the @authors tag. See the copyright.txt in the distribution for a
* full listing of individual contributors.
*
* Licensed 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.jboss.weld.tests.observers.custom;

import static org.junit.Assert.assertTrue;

import javax.enterprise.inject.spi.BeanManager;
import javax.enterprise.inject.spi.Extension;

import org.jboss.arquillian.container.test.api.Deployment;
import org.jboss.arquillian.junit.Arquillian;
import org.jboss.shrinkwrap.api.Archive;
import org.jboss.shrinkwrap.api.BeanArchive;
import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.weld.test.util.Utils;
import org.junit.Test;
import org.junit.runner.RunWith;

/**
*
* @author Martin Kouba
*
*/
@RunWith(Arquillian.class)
public class CustomObserverSuperclassNotifyTest {

@Deployment
public static Archive<?> createTestArchive() {
return ShrinkWrap.create(BeanArchive.class, Utils.getDeploymentNameAsHash(CustomObserverSuperclassNotifyTest.class))
.addPackage(CustomObserverSuperclassNotifyTest.class.getPackage())
.addAsServiceProvider(Extension.class, CustomObserverSuperclassExtension.class);
}

@Test
public void testCustomObserver(BeanManager beanManager) {
beanManager.fireEvent(Long.valueOf(1));
assertTrue(CustomObserverSuperclassExtension.OBSERVED.get());
}
}

0 comments on commit 99afc94

Please sign in to comment.