james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Norman <nor...@apache.org>
Subject Re: svn commit: r998106 - in /james/server/trunk: core-function/src/main/java/org/apache/james/domain/JPADomainList.java user-function/src/main/java/org/apache/james/vut/JPAVirtualUserTable.java
Date Fri, 17 Sep 2010 13:11:41 GMT
Hi Eric,

calling commit() in the finally block a bad idea. It would get executed 
even on a Exception..

Bye
Norman

Am 17.09.2010 14:59, schrieb eric@apache.org:
> Author: eric
> Date: Fri Sep 17 12:59:24 2010
> New Revision: 998106
>
> URL: http://svn.apache.org/viewvc?rev=998106&view=rev
> Log:
> Always start/commit(/rollback) transactions.
>
> Modified:
>      james/server/trunk/core-function/src/main/java/org/apache/james/domain/JPADomainList.java
>      james/server/trunk/user-function/src/main/java/org/apache/james/vut/JPAVirtualUserTable.java
>
> Modified: james/server/trunk/core-function/src/main/java/org/apache/james/domain/JPADomainList.java
> URL: http://svn.apache.org/viewvc/james/server/trunk/core-function/src/main/java/org/apache/james/domain/JPADomainList.java?rev=998106&r1=998105&r2=998106&view=diff
> ==============================================================================
> --- james/server/trunk/core-function/src/main/java/org/apache/james/domain/JPADomainList.java
(original)
> +++ james/server/trunk/core-function/src/main/java/org/apache/james/domain/JPADomainList.java
Fri Sep 17 12:59:24 2010
> @@ -44,6 +44,16 @@ public class JPADomainList extends Abstr
>        */
>       private EntityManagerFactory entityManagerFactory;
>
> +    /**
> +     * Set the entity manager to use.
> +     *
> +     * @param entityManagerFactory
> +     */
> +    @PersistenceUnit
> +    public void setEntityManagerFactory(EntityManagerFactory entityManagerFactory) {
> +        this.entityManagerFactory = entityManagerFactory;
> +    }
> +
>       /*
>        * (non-Javadoc)
>        * @see org.apache.james.lifecycle.Configurable#configure(org.apache.commons.configuration.HierarchicalConfiguration)
> @@ -61,11 +71,17 @@ public class JPADomainList extends Abstr
>       protected List<String>  getDomainListInternal() {
>           List<String>  domains = new ArrayList<String>();
>           EntityManager entityManager = entityManagerFactory.createEntityManager();
> +        final EntityTransaction transaction = entityManager.getTransaction();
>           try {
> +            transaction.begin();
>               domains = entityManager.createNamedQuery("listDomainNames").getResultList();
>           } catch (PersistenceException e) {
>               getLogger().debug("Failed to list domains", e);
> +            if (transaction.isActive()) {
> +                transaction.rollback();
> +            }
>           } finally {
> +            transaction.commit();
>               entityManager.close();
>           }
>           if (domains.size() == 0) {
> @@ -80,12 +96,18 @@ public class JPADomainList extends Abstr
>        */
>       public boolean containsDomain(String domain) {
>           EntityManager entityManager = entityManagerFactory.createEntityManager();
> +        final EntityTransaction transaction = entityManager.getTransaction();
>           try {
> +            transaction.begin();
>               JPADomain jpaDomain = (JPADomain) entityManager.createNamedQuery("findDomainByName").setParameter("name",
domain).getSingleResult();
>               return (jpaDomain != null) ? true : false;
>           } catch (PersistenceException e) {
>               getLogger().debug("Failed to find domain", e);
> +            if (transaction.isActive()) {
> +                transaction.rollback();
> +            }
>           } finally {
> +            transaction.commit();
>               entityManager.close();
>           }
>           return false;
> @@ -101,7 +123,6 @@ public class JPADomainList extends Abstr
>               transaction.begin();
>               JPADomain jpaDomain = new JPADomain(domain);
>               entityManager.persist(jpaDomain);
> -            transaction.commit();
>               return true;
>           } catch (PersistenceException e) {
>               getLogger().debug("Failed to save domain", e);
> @@ -109,6 +130,7 @@ public class JPADomainList extends Abstr
>                   transaction.rollback();
>               }
>           } finally {
> +            transaction.commit();
>               entityManager.close();
>           }
>           return false;
> @@ -123,7 +145,6 @@ public class JPADomainList extends Abstr
>           try {
>               transaction.begin();
>               entityManager.createNamedQuery("deleteDomainByName").setParameter("name",
domain).executeUpdate();
> -            transaction.commit();
>               return true;
>           } catch (PersistenceException e) {
>               getLogger().debug("Failed to remove domain", e);
> @@ -131,19 +152,10 @@ public class JPADomainList extends Abstr
>                   transaction.rollback();
>               }
>           } finally {
> +            transaction.commit();
>               entityManager.close();
>           }
>           return false;
>       }
>
> -    /**
> -     * Set the entity manager to use.
> -     *
> -     * @param entityManagerFactory
> -     */
> -    @PersistenceUnit
> -    public void setEntityManagerFactory(EntityManagerFactory entityManagerFactory) {
> -        this.entityManagerFactory = entityManagerFactory;
> -    }
> -
>   }
>
> Modified: james/server/trunk/user-function/src/main/java/org/apache/james/vut/JPAVirtualUserTable.java
> URL: http://svn.apache.org/viewvc/james/server/trunk/user-function/src/main/java/org/apache/james/vut/JPAVirtualUserTable.java?rev=998106&r1=998105&r2=998106&view=diff
> ==============================================================================
> --- james/server/trunk/user-function/src/main/java/org/apache/james/vut/JPAVirtualUserTable.java
(original)
> +++ james/server/trunk/user-function/src/main/java/org/apache/james/vut/JPAVirtualUserTable.java
Fri Sep 17 12:59:24 2010
> @@ -83,7 +83,6 @@ public class JPAVirtualUserTable extends
>               List<JPAVirtualUser>  virtualUsers = entityManager.createNamedQuery("selectMappings")
>                   .setParameter("user", user)
>                   .setParameter("domain", domain).getResultList();
> -            transaction.commit();
>               if(virtualUsers.size()>  0) {
>                   return virtualUsers.get(0).getTargetAddress();
>               }
> @@ -93,6 +92,7 @@ public class JPAVirtualUserTable extends
>                   transaction.rollback();
>               }
>           } finally {
> +            transaction.commit();
>               entityManager.close();
>           }
>           return null;
> @@ -103,7 +103,9 @@ public class JPAVirtualUserTable extends
>        */
>       protected Collection<String>  getUserDomainMappingsInternal(String user,
String domain) {
>           EntityManager entityManager = entityManagerFactory.createEntityManager();
> +        final EntityTransaction transaction = entityManager.getTransaction();
>           try {
> +            transaction.begin();
>               List<JPAVirtualUser>  virtualUsers = entityManager.createNamedQuery("selectUserDomainMapping")
>                   .setParameter("user", user)
>                   .setParameter("domain", domain).getResultList();
> @@ -112,7 +114,11 @@ public class JPAVirtualUserTable extends
>               }
>           } catch (PersistenceException e) {
>               getLogger().debug("Failed to get user domain mappings", e);
> +            if (transaction.isActive()) {
> +                transaction.rollback();
> +            }
>           } finally {
> +            transaction.commit();
>               entityManager.close();
>           }
>           return null;
> @@ -123,8 +129,10 @@ public class JPAVirtualUserTable extends
>        */
>       protected Map<String,Collection<String>>  getAllMappingsInternal()
{
>           EntityManager entityManager = entityManagerFactory.createEntityManager();
> +        final EntityTransaction transaction = entityManager.getTransaction();
>           Map<String,Collection<String>>  mapping = new HashMap<String,Collection<String>>();
>           try {
> +            transaction.begin();
>               List<JPAVirtualUser>  virtualUsers = entityManager.createNamedQuery("selectAllMappings").getResultList();
>               for (JPAVirtualUser virtualUser: virtualUsers) {
>                   mapping.put(virtualUser.getUser()+ "@" + virtualUser.getDomain(), VirtualUserTableUtil.mappingToCollection(virtualUser.getTargetAddress()));
> @@ -132,7 +140,11 @@ public class JPAVirtualUserTable extends
>               if (mapping.size()>  0) return mapping;
>           } catch (PersistenceException e) {
>               getLogger().debug("Failed to get all mappings", e);
> +            if (transaction.isActive()) {
> +                transaction.rollback();
> +            }
>           } finally {
> +            transaction.commit();
>               entityManager.close();
>           }
>           return null;
> @@ -170,7 +182,6 @@ public class JPAVirtualUserTable extends
>                   .setParameter("targetAddress", mapping)
>                   .setParameter("user", user)
>                   .setParameter("domain", domain).executeUpdate();
> -            transaction.commit();
>               if (updated>  0) {
>                   return true;
>               }
> @@ -180,6 +191,7 @@ public class JPAVirtualUserTable extends
>                   transaction.rollback();
>               }
>           } finally {
> +            transaction.commit();
>               entityManager.close();
>           }
>           return false;
> @@ -203,7 +215,6 @@ public class JPAVirtualUserTable extends
>                   .setParameter("user", user)
>                   .setParameter("domain", domain)
>                   .setParameter("targetAddress", mapping).executeUpdate();
> -            transaction.commit();
>               if (deleted>  0) {
>                   return true;
>               }
> @@ -213,6 +224,7 @@ public class JPAVirtualUserTable extends
>                   transaction.rollback();
>               }
>           } finally {
> +            transaction.commit();
>               entityManager.close();
>           }
>           return false;
> @@ -233,7 +245,6 @@ public class JPAVirtualUserTable extends
>               transaction.begin();
>               JPAVirtualUser jpaVirtualUser = new JPAVirtualUser(user, domain, mapping);
>               entityManager.persist(jpaVirtualUser);
> -            transaction.commit();
>               return true;
>           } catch (PersistenceException e) {
>               getLogger().debug("Failed to save virtual user", e);
> @@ -241,6 +252,7 @@ public class JPAVirtualUserTable extends
>                   transaction.rollback();
>               }
>           } finally {
> +            transaction.commit();
>               entityManager.close();
>           }
>           return false;
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>    


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message