hi mark, there is no need to use a different client. it's just the order of doing it. originally i planned something different which would have resulted in the same. at the end of the refactoring it was more like the original version, but i didn't revert the whole work just for a simpler diff of 2 classes. if we need to do it, we should discuss it for myfaces as a whole.
regards, gerhard http://www.irian.at Your JSF powerhouse - JSF Consulting, Development and Courses in English and German Professional Support for Apache MyFaces 2011/11/4 Mark Struberg <[email protected]>: > I'll revert that and do it cleanly. > > And you should check your svn client, Idea apparently trashes the svn > history... > > LieGrue, > strub > > > > ----- Original Message ----- >> From: "[email protected]" <[email protected]> >> To: [email protected] >> Cc: >> Sent: Friday, November 4, 2011 2:11 AM >> Subject: svn commit: r1197378 - in >> /myfaces/extensions/cdi/trunk/jee-modules/jpa-module: api/ >> api/src/main/java/org/apache/myfaces/extensions/cdi/jpa/api/datasource/ >> impl/src/main/java/org/apache/myfaces/extensions/cdi/jpa/impl/datasource/ >> >> Author: gpetracek >> Date: Fri Nov 4 01:11:33 2011 >> New Revision: 1197378 >> >> URL: http://svn.apache.org/viewvc?rev=1197378&view=rev >> Log: >> EXTCDI-236 AbstractConfigurableDataSource and refactoring to existing >> util-methods >> >> Added: >> >> myfaces/extensions/cdi/trunk/jee-modules/jpa-module/api/src/main/java/org/apache/myfaces/extensions/cdi/jpa/api/datasource/AbstractConfigurableDataSource.java >> >> myfaces/extensions/cdi/trunk/jee-modules/jpa-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jpa/impl/datasource/ >> >> myfaces/extensions/cdi/trunk/jee-modules/jpa-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jpa/impl/datasource/DefaultConfigurableDataSource.java >> (contents, props changed) >> - copied, changed from r1197347, >> myfaces/extensions/cdi/trunk/jee-modules/jpa-module/api/src/main/java/org/apache/myfaces/extensions/cdi/jpa/api/datasource/ConfigurableDataSource.java >> Removed: >> >> myfaces/extensions/cdi/trunk/jee-modules/jpa-module/api/src/main/java/org/apache/myfaces/extensions/cdi/jpa/api/datasource/ConfigurableDataSource.java >> Modified: >> myfaces/extensions/cdi/trunk/jee-modules/jpa-module/api/pom.xml >> >> Modified: myfaces/extensions/cdi/trunk/jee-modules/jpa-module/api/pom.xml >> URL: >> http://svn.apache.org/viewvc/myfaces/extensions/cdi/trunk/jee-modules/jpa-module/api/pom.xml?rev=1197378&r1=1197377&r2=1197378&view=diff >> ============================================================================== >> --- myfaces/extensions/cdi/trunk/jee-modules/jpa-module/api/pom.xml >> (original) >> +++ myfaces/extensions/cdi/trunk/jee-modules/jpa-module/api/pom.xml Fri Nov >> 4 >> 01:11:33 2011 >> @@ -36,18 +36,6 @@ >> >> <groupId>org.apache.myfaces.extensions.cdi.core</groupId> >> <artifactId>myfaces-extcdi-core-api</artifactId> >> </dependency> >> - <dependency> >> - <groupId>commons-beanutils</groupId> >> - <artifactId>commons-beanutils-core</artifactId> >> - <version>1.8.3</version> >> - <optional>true</optional> >> - </dependency> >> - <dependency> >> - <groupId>commons-logging</groupId> >> - <artifactId>commons-logging</artifactId> >> - <version>1.1.1</version> >> - <optional>true</optional> >> - </dependency> >> </dependencies> >> >> <build> >> @@ -66,39 +54,6 @@ >> </execution> >> </executions> >> </plugin> >> - <plugin> >> - <groupId>org.apache.maven.plugins</groupId> >> - <artifactId>maven-shade-plugin</artifactId> >> - <executions> >> - <execution> >> - <phase>package</phase> >> - <goals> >> - <goal>shade</goal> >> - </goals> >> - </execution> >> - </executions> >> - <configuration> >> - <createSourcesJar>true</createSourcesJar> >> - >> <createDependencyReducedPom>false</createDependencyReducedPom> >> - <minimizeJar>true</minimizeJar> >> - <artifactSet> >> - <includes> >> - >> <include>commons-beanutils:commons-beanutils</include> >> - >> <include>commons-logging:commons-logging</include> >> - </includes> >> - </artifactSet> >> - <relocations> >> - <relocation> >> - >> <pattern>org.apache.commons.beanutils</pattern> >> - >> <shadedPattern>org.apache.myfaces.codi.shaded.commons.beanutils</shadedPattern> >> - </relocation> >> - <relocation> >> - >> <pattern>org.apache.commons.logging</pattern> >> - >> <shadedPattern>org.apache.myfaces.codi.shaded.commons.logging</shadedPattern> >> - </relocation> >> - </relocations> >> - </configuration> >> - </plugin> >> </plugins> >> </build> >> >> >> Added: >> myfaces/extensions/cdi/trunk/jee-modules/jpa-module/api/src/main/java/org/apache/myfaces/extensions/cdi/jpa/api/datasource/AbstractConfigurableDataSource.java >> URL: >> http://svn.apache.org/viewvc/myfaces/extensions/cdi/trunk/jee-modules/jpa-module/api/src/main/java/org/apache/myfaces/extensions/cdi/jpa/api/datasource/AbstractConfigurableDataSource.java?rev=1197378&view=auto >> ============================================================================== >> --- >> myfaces/extensions/cdi/trunk/jee-modules/jpa-module/api/src/main/java/org/apache/myfaces/extensions/cdi/jpa/api/datasource/AbstractConfigurableDataSource.java >> (added) >> +++ >> myfaces/extensions/cdi/trunk/jee-modules/jpa-module/api/src/main/java/org/apache/myfaces/extensions/cdi/jpa/api/datasource/AbstractConfigurableDataSource.java >> Fri Nov 4 01:11:33 2011 >> @@ -0,0 +1,191 @@ >> +/* >> + * 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.myfaces.extensions.cdi.jpa.api.datasource; >> + >> +import >> org.apache.myfaces.extensions.cdi.core.api.provider.BeanManagerProvider; >> + >> +import javax.sql.DataSource; >> +import java.io.PrintWriter; >> +import java.sql.Connection; >> +import java.sql.SQLException; >> +import java.sql.SQLFeatureNotSupportedException; >> +import java.util.logging.Logger; >> + >> +/** >> + * <p>This class can be used instead of a real DataSource. >> + * It is a simple wrapper to hide any database configuration details >> + * and make it configurable via CDI.</p> >> + * <p/> >> + * <p>The configuration itself will be provided via CDI mechanics. >> + * To distinguish different databases, users can specify a >> + * <code>connectionId</code>. If no >> <code>connectionId</code> is set, >> + * the String <code>default</code> will be used</p> >> + */ >> +public abstract class AbstractConfigurableDataSource implements DataSource >> +{ >> + /** >> + * config and settings are loaded only once. >> + */ >> + private boolean loaded; >> + >> + /** >> + * The connectionId allows to configure multiple databases. >> + * This can e.g. be used to distinguish between a 'customer' and >> 'admin' >> + * database. >> + */ >> + protected String connectionId = "default"; >> + >> + /** >> + * The underlying configuration of the datasource >> + */ >> + private DataSourceConfig dataSourceConfig; >> + >> + private volatile DataSource wrappedDataSource; >> + >> + public AbstractConfigurableDataSource() >> + { >> + loaded = false; >> + dataSourceConfig = >> BeanManagerProvider.getInstance().getContextualReference(DataSourceConfig.class); >> + } >> + >> + public void setConnectionId(String connectionId) >> + { >> + if (loaded) >> + { >> + throw new IllegalStateException("connectionId must not get >> changed after the DataSource was established"); >> + } >> + this.connectionId = connectionId; >> + } >> + >> + /** >> + * {@inheritDoc} >> + */ >> + public Connection getConnection() throws SQLException >> + { >> + return getConnection(null, null); >> + } >> + >> + /** >> + * {@inheritDoc} >> + */ >> + public Connection getConnection(String userName, String password) throws >> SQLException >> + { >> + if (wrappedDataSource == null) >> + { >> + initDataSource(); >> + } >> + >> + if (userName == null && password == null) >> + { >> + return wrappedDataSource.getConnection(); >> + } >> + return wrappedDataSource.getConnection(userName, password); >> + } >> + >> + >> + /** >> + * {@inheritDoc} >> + */ >> + public PrintWriter getLogWriter() throws SQLException >> + { >> + return null; >> + } >> + >> + /** >> + * {@inheritDoc} >> + */ >> + public void setLogWriter(PrintWriter printWriter) throws SQLException >> + { >> + } >> + >> + /** >> + * {@inheritDoc} >> + */ >> + public void setLoginTimeout(int loginTimeout) throws SQLException >> + { >> + } >> + >> + /** >> + * {@inheritDoc} >> + */ >> + public int getLoginTimeout() throws SQLException >> + { >> + return 0; >> + } >> + >> + /** >> + * {@inheritDoc} >> + */ >> + public <T> T unwrap(Class<T> iface) throws SQLException >> + { >> + if (isWrapperFor(iface)) >> + { >> + return (T) this; >> + } >> + else >> + { >> + return null; >> + } >> + } >> + >> + /** >> + * {@inheritDoc} >> + */ >> + public boolean isWrapperFor(Class<?> iface) throws SQLException >> + { >> + return iface.isAssignableFrom(getClass()); >> + } >> + >> + /** >> + * NEW JDK1.7 signature. >> + * This makes sure that CODI can also get compiled using java-7. >> + * This method is not actively used though. >> + */ >> + public Logger getParentLogger() throws SQLFeatureNotSupportedException >> + { >> + throw new SQLFeatureNotSupportedException(); >> + } >> + >> + protected void initDataSource() throws SQLException >> + { >> + // double check lock idiom on volatile member is ok as of Java5 >> + if (wrappedDataSource != null) >> + { >> + return; >> + } >> + >> + this.wrappedDataSource = resolveDataSource(); >> + >> + if(this.wrappedDataSource == null) >> + { >> + throw new IllegalStateException("No DataSource found."); >> + } >> + >> + configureDataSource(this.wrappedDataSource); >> + } >> + >> + public DataSourceConfig getDataSourceConfig() >> + { >> + return dataSourceConfig; >> + } >> + >> + protected abstract DataSource resolveDataSource() throws SQLException; >> + >> + protected abstract void configureDataSource(DataSource dataSource); >> +} >> >> Copied: >> myfaces/extensions/cdi/trunk/jee-modules/jpa-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jpa/impl/datasource/DefaultConfigurableDataSource.java >> (from r1197347, >> myfaces/extensions/cdi/trunk/jee-modules/jpa-module/api/src/main/java/org/apache/myfaces/extensions/cdi/jpa/api/datasource/ConfigurableDataSource.java) >> URL: >> http://svn.apache.org/viewvc/myfaces/extensions/cdi/trunk/jee-modules/jpa-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jpa/impl/datasource/DefaultConfigurableDataSource.java?p2=myfaces/extensions/cdi/trunk/jee-modules/jpa-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jpa/impl/datasource/DefaultConfigurableDataSource.java&p1=myfaces/extensions/cdi/trunk/jee-modules/jpa-module/api/src/main/java/org/apache/myfaces/extensions/cdi/jpa/api/datasource/ConfigurableDataSource.java&r1=1197347&r2=1197378&rev=1197378&view=diff >> ============================================================================== >> --- >> myfaces/extensions/cdi/trunk/jee-modules/jpa-module/api/src/main/java/org/apache/myfaces/extensions/cdi/jpa/api/datasource/ConfigurableDataSource.java >> (original) >> +++ >> myfaces/extensions/cdi/trunk/jee-modules/jpa-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jpa/impl/datasource/DefaultConfigurableDataSource.java >> Fri Nov 4 01:11:33 2011 >> @@ -16,206 +16,94 @@ >> * specific language governing permissions and limitations >> * under the License. >> */ >> -package org.apache.myfaces.extensions.cdi.jpa.api.datasource; >> +package org.apache.myfaces.extensions.cdi.jpa.impl.datasource; >> >> -import org.apache.commons.beanutils.BeanUtils; >> -import >> org.apache.myfaces.extensions.cdi.core.api.provider.BeanManagerProvider; >> +import org.apache.myfaces.extensions.cdi.core.api.util.ClassUtils; >> +import org.apache.myfaces.extensions.cdi.core.impl.util.JndiUtils; >> +import >> org.apache.myfaces.extensions.cdi.jpa.api.datasource.AbstractConfigurableDataSource; >> >> -import javax.naming.InitialContext; >> import javax.sql.DataSource; >> -import java.io.PrintWriter; >> -import java.sql.Connection; >> +import java.lang.reflect.Method; >> import java.sql.SQLException; >> -import java.sql.SQLFeatureNotSupportedException; >> import java.util.Map; >> +import java.util.logging.Level; >> import java.util.logging.Logger; >> -import javax.naming.Context; >> >> -/** >> - * <p>This class can be used instead of a real DataSource. >> - * It is a simple wrapper to hide any database configuration details >> - * and make it configurable via CDI.</p> >> - * >> - * <p>The configuration itself will be provided via CDI mechanics. >> - * To distinguish different databases, users can specify a >> - * <code>connectionId</code>. If no >> <code>connectionId</code> is set, >> - * the String <code>default</code> will be used</p> >> - */ >> -public class ConfigurableDataSource implements DataSource >> +public class DefaultConfigurableDataSource extends >> AbstractConfigurableDataSource >> { >> /** >> - * config and settings are loaded only once. >> - */ >> - private boolean loaded; >> - >> - /** >> - * The connectionId allows to configure multiple databases. >> - * This can e.g. be used to distinguish between a 'customer' and >> 'admin' >> - * database. >> - */ >> - private String connectionId = "default"; >> - >> - /** >> - * The underlying configuration of the datasource >> - */ >> - private DataSourceConfig dataSourceConfig; >> - >> - /** >> - * >> + * {@inheritDoc} >> */ >> - private volatile DataSource wrappedDataSource; >> - >> - public ConfigurableDataSource() >> - { >> - loaded = false; >> - dataSourceConfig = >> BeanManagerProvider.getInstance().getContextualReference(DataSourceConfig.class); >> - } >> - >> - public void setConnectionId(String connectionId) >> - { >> - if (loaded) >> - { >> - throw new IllegalStateException("connectionId must not get >> changed after the DataSource was established"); >> - } >> - this.connectionId = connectionId; >> - } >> - >> - public Connection getConnection() throws SQLException >> + protected DataSource resolveDataSource() throws SQLException >> { >> - return getConnection(null, null); >> - } >> + String jndiLookupName = >> getDataSourceConfig().getJndiResourceName(connectionId); >> >> - public Connection getConnection(String userName, String password) throws >> SQLException >> - { >> - if (wrappedDataSource == null) >> + if (jndiLookupName != null && jndiLookupName.length() > 0) >> { >> - initDataSource(); >> + return JndiUtils.lookup(jndiLookupName, DataSource.class); >> } >> >> - if (userName == null && password == null) >> + // no JNDI, so we take the direct JDBC route. >> + String jdbcDriverClass = >> getDataSourceConfig().getDriverClassName(connectionId); >> + if (jdbcDriverClass == null || jdbcDriverClass.length() == 0) >> { >> - return wrappedDataSource.getConnection(); >> + throw new SQLException("Neither a JNDI location nor a JDBC >> driver class name is configured!"); >> } >> - return wrappedDataSource.getConnection(userName, password); >> - } >> - >> - >> - public PrintWriter getLogWriter() throws SQLException >> - { >> - return null; >> - } >> >> - public void setLogWriter(PrintWriter printWriter) throws SQLException >> - { >> + return ClassUtils.tryToInstantiateClassForName(jdbcDriverClass, >> DataSource.class); >> } >> >> - public void setLoginTimeout(int loginTimeout) throws SQLException >> - { >> - } >> - >> - public int getLoginTimeout() throws SQLException >> + /** >> + * {@inheritDoc} >> + */ >> + protected void configureDataSource(DataSource dataSource) >> { >> - return 0; >> - } >> + Map<String, String> config = >> getDataSourceConfig().getConnectionProperties(connectionId); >> >> - public <T> T unwrap(Class<T> iface) throws SQLException >> - { >> - if (isWrapperFor(iface)) >> - { >> - return (T) this; >> - } >> - else >> + for (Map.Entry<String, String> configOption : config.entrySet()) >> { >> - return null; >> + setProperty(dataSource, configOption); >> } >> } >> >> - public boolean isWrapperFor(Class<?> iface) throws SQLException >> + protected void setProperty(DataSource dataSource, Map.Entry<String, >> String> configOption) >> { >> - return iface.isAssignableFrom(ConfigurableDataSource.class); >> - } >> - >> - /** >> - * NEW JDK1.7 signature. >> - * This makes sure that CODI can also get compiled using java-7. >> - * This method is not actively used though. >> - */ >> - public Logger getParentLogger() throws SQLFeatureNotSupportedException >> - { >> - throw new SQLFeatureNotSupportedException(); >> - } >> + Method setterMethod = findSetterForProperty(dataSource, >> configOption); >> >> - /** >> - * >> - */ >> - protected void initDataSource() throws SQLException >> - { >> - // double check lock idiom on volatile member is ok as of Java5 >> - if (wrappedDataSource != null) >> + if(setterMethod == null) >> { >> - return; >> - } >> + Logger logger = Logger.getLogger(getClass().getName()); >> >> - String jndiLookupName = >> dataSourceConfig.getJndiResourceName(connectionId); >> - if (jndiLookupName != null && jndiLookupName.length() > 0) >> - { >> - wrappedDataSource = resolveDataSourceViaJndi(jndiLookupName); >> + if(logger.isLoggable(Level.WARNING)) >> + { >> + logger.warning(dataSource.getClass().getName() + >> + " has no setter for property '" + >> configOption.getKey() + "'. Property gets ignored."); >> + } >> return; >> } >> >> - >> - // no JNDI, so we take the direct JDBC route. >> - String jdbcDriverClass = >> dataSourceConfig.getDriverClassName(connectionId); >> - if (jdbcDriverClass == null && jdbcDriverClass.length() == 0) >> - { >> - throw new SQLException("Neither a JNDI location nor a JDBC >> driver class name is configured!"); >> - } >> - >> try >> { >> - Class clazz = Class.forName(jdbcDriverClass); >> - >> - // the given driver classname must be a DataSource >> - if (!DataSource.class.isAssignableFrom(clazz)) >> - { >> - throw new SQLException("Configured DriverClassName is not >> a javax.sql.DataSource: " >> - + jdbcDriverClass); >> - } >> - >> - wrappedDataSource = (DataSource) clazz.newInstance(); >> - >> - Map<String, String> config = >> dataSourceConfig.getConnectionProperties(connectionId); >> - for (Map.Entry<String, String> configOption : >> config.entrySet()) >> - { >> - BeanUtils.setProperty(wrappedDataSource, >> configOption.getKey(), >> configOption.getValue()); >> - } >> + setterMethod.invoke(dataSource, configOption.getValue()); >> } >> catch (Exception e) >> { >> - wrappedDataSource = null; >> - >> - if (e instanceof SQLException) >> - { >> - throw (SQLException) e; >> - } >> - throw new SQLException(e); >> + throw new >> IllegalStateException(setterMethod.getDeclaringClass().getName() + "#" >> + setterMethod.getName() + >> + " failed", e); >> } >> } >> >> - protected DataSource resolveDataSourceViaJndi(String jndiLookupName) >> + protected Method findSetterForProperty(DataSource dataSource, >> Map.Entry<String, String> configOption) >> { >> - DataSource ds = null; >> - try >> - { >> - Context jndiContext = new InitialContext(); >> - ds = (DataSource) jndiContext.lookup(jndiLookupName); >> - } >> - catch (Exception e) >> + for (Method method : dataSource.getClass().getMethods()) >> { >> - throw new IllegalArgumentException("Could not lookup >> DataSource from JNDI using " + jndiLookupName); >> + if (method.getParameterTypes().length == 1 && >> + >> String.class.isAssignableFrom(method.getParameterTypes()[0]) && >> + method.getName().equalsIgnoreCase("set" + >> configOption.getKey())) //simple detection >> + { >> + return method; >> + } >> } >> - >> - return ds; >> + return null; >> } >> - >> } >> >> Propchange: >> myfaces/extensions/cdi/trunk/jee-modules/jpa-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jpa/impl/datasource/DefaultConfigurableDataSource.java >> ------------------------------------------------------------------------------ >> svn:eol-style = native >> >
