myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gerhard Petracek <gerhard.petra...@gmail.com>
Subject Re: 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/
Date Fri, 04 Nov 2011 10:18:40 GMT
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 <struberg@yahoo.de>:
> 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: "gpetracek@apache.org" <gpetracek@apache.org>
>> To: commits@myfaces.apache.org
>> 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
>>
>

Mime
View raw message