TOMEE-1817 avoid NPE if a RA has a getter which is not supposed to be handled by the container
Project: http://git-wip-us.apache.org/repos/asf/tomee/repo Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/513df6fb Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/513df6fb Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/513df6fb Branch: refs/heads/tomee-1.7.x Commit: 513df6fba20342a65d44e71b6a973202b4dc257a Parents: 72927b1 Author: Romain manni-Bucau <rmannibu...@gmail.com> Authored: Fri May 20 08:49:51 2016 +0200 Committer: Jonathan Gallimore <j...@jrg.me.uk> Committed: Mon May 30 20:39:26 2016 +0100 ---------------------------------------------------------------------- .../openejb/config/AnnotationDeployer.java | 30 ++- .../openejb/config/GetterConnectorTest.java | 250 +++++++++++++++++++ 2 files changed, 267 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tomee/blob/513df6fb/container/openejb-core/src/main/java/org/apache/openejb/config/AnnotationDeployer.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/config/AnnotationDeployer.java b/container/openejb-core/src/main/java/org/apache/openejb/config/AnnotationDeployer.java index 911a3a8..04e240f 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/config/AnnotationDeployer.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/config/AnnotationDeployer.java @@ -844,24 +844,29 @@ public class AnnotationDeployer implements DynamicDeployer { } if (!containsConfigProperty(configProperties, name)) { - final ConfigProperty configProperty = new ConfigProperty(); configProperties.add(configProperty); Object value = null; - try { - value = propertyDescriptor.getReadMethod().invoke(o); - } catch (final Exception e) { - // no-op + if (propertyDescriptor.getReadMethod() != null) { + try { + value = propertyDescriptor.getReadMethod().invoke(o); + } catch (final Exception e) { + // no-op + } } - javax.resource.spi.ConfigProperty annotation = propertyDescriptor.getWriteMethod().getAnnotation(javax.resource.spi.ConfigProperty.class); - if (annotation == null) { - try { - // if there's no annotation on the setter, we'll try and scrape one off the field itself (assuming the same name) - annotation = clazz.getDeclaredField(name).getAnnotation(javax.resource.spi.ConfigProperty.class); - } catch (final Exception ignored) { - // no-op : getDeclaredField() throws exceptions and does not return null + final Method write = propertyDescriptor.getWriteMethod(); + javax.resource.spi.ConfigProperty annotation = null; + if (write != null) { + annotation = write.getAnnotation(javax.resource.spi.ConfigProperty.class); + if (annotation == null) { + try { + // if there's no annotation on the setter, we'll try and scrape one off the field itself (assuming the same name) + annotation = clazz.getDeclaredField(name).getAnnotation(javax.resource.spi.ConfigProperty.class); + } catch (final Exception ignored) { + // no-op : getDeclaredField() throws exceptions and does not return null + } } } @@ -880,7 +885,6 @@ public class AnnotationDeployer implements DynamicDeployer { configProperty.setConfigPropertySupportsDynamicUpdates(annotation.supportsDynamicUpdates()); configProperty.setDescriptions(stringsToTexts(annotation.description())); } - } } http://git-wip-us.apache.org/repos/asf/tomee/blob/513df6fb/container/openejb-core/src/test/java/org/apache/openejb/config/GetterConnectorTest.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/test/java/org/apache/openejb/config/GetterConnectorTest.java b/container/openejb-core/src/test/java/org/apache/openejb/config/GetterConnectorTest.java new file mode 100644 index 0000000..7e67331 --- /dev/null +++ b/container/openejb-core/src/test/java/org/apache/openejb/config/GetterConnectorTest.java @@ -0,0 +1,250 @@ +/* + * 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.openejb.config; + +import org.apache.openejb.jee.ConfigProperty; +import org.apache.openejb.jee.ConnectionDefinition; +import org.apache.openejb.jee.Connector; +import org.apache.openejb.jee.OutboundResourceAdapter; +import org.apache.openejb.jee.ResourceAdapter; +import org.apache.openejb.junit.ApplicationComposer; +import org.apache.openejb.loader.SystemInstance; +import org.apache.openejb.spi.ContainerSystem; +import org.apache.openejb.testing.Module; +import org.apache.openejb.testing.SimpleLog; +import org.junit.Test; +import org.junit.runner.RunWith; + +import javax.annotation.Resource; +import javax.naming.NamingException; +import javax.resource.ResourceException; +import javax.resource.cci.ConnectionFactory; +import javax.resource.spi.ActivationSpec; +import javax.resource.spi.BootstrapContext; +import javax.resource.spi.ConnectionEventListener; +import javax.resource.spi.ConnectionManager; +import javax.resource.spi.ConnectionRequestInfo; +import javax.resource.spi.LocalTransaction; +import javax.resource.spi.ManagedConnection; +import javax.resource.spi.ManagedConnectionFactory; +import javax.resource.spi.ManagedConnectionMetaData; +import javax.resource.spi.ResourceAdapterInternalException; +import javax.resource.spi.ValidatingManagedConnectionFactory; +import javax.resource.spi.endpoint.MessageEndpointFactory; +import javax.security.auth.Subject; +import javax.transaction.xa.XAResource; +import java.io.PrintWriter; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +@SimpleLog +@RunWith(ApplicationComposer.class) +public class GetterConnectorTest { + @Module + public Connector connector() { + final ConfigProperty configProperty = new ConfigProperty(); + configProperty.setConfigPropertyName("conf"); + configProperty.setConfigPropertyType(String.class.getName()); + configProperty.setConfigPropertyValue("GetterConnectorTest"); + + final ConnectionDefinition connectionDefinition = new ConnectionDefinition(); + connectionDefinition.setConnectionFactoryImplClass(MyMcf.class.getName()); + connectionDefinition.setConnectionInterface(ConnectionFactory.class.getName()); + + final OutboundResourceAdapter out = new OutboundResourceAdapter(); + out.getConnectionDefinition().add(connectionDefinition); + + final ResourceAdapter ra = new ResourceAdapter(); + ra.setResourceAdapterClass(MyRa.class.getName()); + ra.getConfigProperty().add(configProperty); + + final Connector connector = new Connector(); + connector.setVersion("1.7"); + connector.setResourceAdapter(ra); + return connector; + } + + @Test + public void run() throws NamingException { + // https://issues.apache.org/jira/browse/TOMEE-1817 is a NPE so if started we are good + final MyRa ra = MyRa.class.cast(SystemInstance.get().getComponent(ContainerSystem.class).getJNDIContext().lookup("openejb:Resource/connectorRA")); + assertNotNull(ra); + assertEquals("GetterConnectorTest", ra.getConf()); + } + + public static class MyRa implements javax.resource.spi.ResourceAdapter { + @javax.resource.spi.ConfigProperty + private String conf; + + private String TOMEE1817; + + public String getTOMEE1817() { // getter without setter + return TOMEE1817; + } + + public String getConf() { + return conf; + } + + public void setConf(final String conf) { + this.conf = conf; + } + + @Override + public void start(final BootstrapContext ctx) throws ResourceAdapterInternalException { + // no-op + } + + @Override + public void stop() { + // no-op + } + + @Override + public void endpointActivation(final MessageEndpointFactory endpointFactory, final ActivationSpec spec) throws ResourceException { + // no-op + } + + @Override + public void endpointDeactivation(final MessageEndpointFactory endpointFactory, final ActivationSpec spec) { + // no-op + } + + @Override + public XAResource[] getXAResources(final ActivationSpec[] specs) throws ResourceException { + return new XAResource[0]; + } + } + public static class MyMcf implements ManagedConnectionFactory, ValidatingManagedConnectionFactory { + private final Set<ManagedConnection> connections = new HashSet<>(); + private final AtomicBoolean evicted = new AtomicBoolean(false); + private final AtomicBoolean destroyed = new AtomicBoolean(false); + + @Override + public Object createConnectionFactory(final ConnectionManager cxManager) throws ResourceException { + return null; + } + + @Override + public Object createConnectionFactory() throws ResourceException { + return null; + } + + @Override + public ManagedConnection createManagedConnection(final Subject subject, final ConnectionRequestInfo cxRequestInfo) throws ResourceException { + return new ManagedConnection() { + @Override + public Object getConnection(Subject subject, ConnectionRequestInfo cxRequestInfo) throws ResourceException { + connections.add(this); + return this; + } + + @Override + public void destroy() throws ResourceException { + connections.remove(this); + destroyed.set(true); + } + + @Override + public void cleanup() throws ResourceException { + // no-op + } + + @Override + public void associateConnection(Object connection) throws ResourceException { + // no-op + } + + @Override + public void addConnectionEventListener(ConnectionEventListener listener) { + // no-op + } + + @Override + public void removeConnectionEventListener(ConnectionEventListener listener) { + // no-op + } + + @Override + public XAResource getXAResource() throws ResourceException { + return null; + } + + @Override + public LocalTransaction getLocalTransaction() throws ResourceException { + return new LocalTransaction() { + @Override + public void begin() throws ResourceException { + + } + + @Override + public void commit() throws ResourceException { + + } + + @Override + public void rollback() throws ResourceException { + + } + }; + } + + @Override + public ManagedConnectionMetaData getMetaData() throws ResourceException { + return null; + } + + @Override + public void setLogWriter(PrintWriter out) throws ResourceException { + // no-op + } + + @Override + public PrintWriter getLogWriter() throws ResourceException { + return null; + } + }; + } + + @Override + public ManagedConnection matchManagedConnections(final Set connectionSet, final Subject subject, + final ConnectionRequestInfo cxRequestInfo) throws ResourceException { + return null; + } + + @Override + public void setLogWriter(PrintWriter out) throws ResourceException { + // no-op + } + + @Override + public PrintWriter getLogWriter() throws ResourceException { + return null; + } + + @Override + public Set getInvalidConnections(final Set connectionSet) throws ResourceException { + evicted.set(true); + return connections; + } + } +}
