OPENJPA-118: Implemented patch provided by David Ezzio for broken openjpa.AutoDetach behavior

git-svn-id: https://svn.apache.org/repos/asf/incubator/openjpa/trunk@502022 13f79535-47bb-0310-9956-ffa450edef68

openjpa-kernel/src/main/java/org/apache/openjpa/conf/AutoDetachValue.java

@@ -15,6 +15,9 @@
  */
 package org.apache.openjpa.conf;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import org.apache.openjpa.kernel.AutoDetach;
 import org.apache.openjpa.lib.conf.StringListValue;
 
@@ -36,8 +39,8 @@ class AutoDetachValue
     private static String[] ALIASES = new String[]{
         DETACH_CLOSE, String.valueOf(AutoDetach.DETACH_CLOSE),
         DETACH_COMMIT, String.valueOf(AutoDetach.DETACH_COMMIT),
-        DETACH_ROLLBACK, String.valueOf(AutoDetach.DETACH_ROLLBACK),
         DETACH_NONTXREAD, String.valueOf(AutoDetach.DETACH_NONTXREAD),
+        DETACH_ROLLBACK, String.valueOf(AutoDetach.DETACH_ROLLBACK),
         // for compatibility with JDO DetachAllOnCommit
         "true", String.valueOf(AutoDetach.DETACH_COMMIT),
         "false", "0",
@@ -70,4 +73,13 @@ class AutoDetachValue
             
         return _flags;
     }
+
+    protected List getAliasList() {
+        // We do not document the numeric values and they are not
+        // helpful to someone trying to understand the error message
+        ArrayList list = new ArrayList();
+        for (int x = 0; x < ALIASES.length; x += 2)
+            list.add(ALIASES[x]);
+        return list;
+    }
 }

openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/StringListValue.java

@@ -15,6 +15,13 @@
  */
 package org.apache.openjpa.lib.conf;
 
+import java.util.Arrays;
+import java.util.List;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.openjpa.lib.util.Localizer;
+import org.apache.openjpa.lib.util.ParseException;
+
 import serp.util.Strings;
 
 /**
@@ -25,6 +32,8 @@ import serp.util.Strings;
 public class StringListValue extends Value {
 
     public static final String[] EMPTY = new String[0];
+    private static final Localizer s_loc = Localizer.forPackage
+        (StringListValue.class);
 
     private String[] _values = EMPTY;
 
@@ -51,20 +60,82 @@ public class StringListValue extends Value {
         return String[].class;
     }
     
+    /**
+     * Unalias the value list.  This method defers to super.unalias()
+     * UNLESS the string passed is a list of values for a property that
+     * has aliases.
+     */
+    public String unalias(String str) {
+        
+        // defer to super.unalias
+        String[] aliases = getAliases();
+        if (aliases.length <= 0 || str == null)
+            return super.unalias(str);
+        str = str.trim();
+        if (str.length() <= 0)
+            return super.unalias(str);
+        
+        // snag this case early as it only causes problems
+        if (str.equals(","))
+            throw new ParseException(s_loc.get("invalid-list-config",
+                getProperty(), str, getAliasList()));
+        
+        // unalias the list and concatenate the list of
+        // canonical values.  Also, catch any bad aliases.
+        boolean found;
+        String iString;
+        StringBuffer retv = new StringBuffer();
+        String[] vals = str.split(",", 0);
+        
+        for (int i = 0; i < vals.length; i++) {
+            iString = vals[i] = vals[i].trim();
+            
+            found = false;
+            if (i > 0)
+                retv.append(',');
+            
+            for (int x = 0; x < aliases.length; x += 2)
+                if (StringUtils.equals(iString, aliases[x])
+                    || StringUtils.equals(iString, aliases[x + 1])) {
+                    retv.append(aliases[x + 1]);
+                    found = true;
+                    break;
+                }
+            
+            // If the alias list is not comprehensive, add any unknown
+            // values back onto the list
+            if (!found) {
+                if (isAliasListComprehensive())
+                    throw new ParseException(s_loc.get("invalid-list-config",
+                        getProperty(), str, getAliasList()));
+                else
+                    retv.append(iString);
+            }
+        }
+        return retv.toString();
+    }
+
     protected String getInternalString() {
         return Strings.join(_values, ", ");
     }
 
     protected void setInternalString(String val) {
         String[] vals = Strings.split(val, ",", 0);
-        if (vals != null)
+        if (vals != null) {
             for (int i = 0; i < vals.length; i++)
                 vals[i] = vals[i].trim();
+        }
+
         set(vals);
     }
 
     protected void setInternalObject(Object obj) {
         set((String[]) obj);
     }
+    
+    protected List getAliasList() {
+        return Arrays.asList(getAliases());
+    }
+    
 }
 

openjpa-lib/src/main/resources/org/apache/openjpa/lib/conf/localizer.properties

@@ -35,6 +35,10 @@ invalid-config-param-hint: There was an error while setting up the \
 invalid-enumerated-config: There was an error while setting up the \
 	configuration option "{0}", and it was set to "{1}". All \
 	possible values for this setting are: {2}.
+invalid-list-config: There was an error setting up the \
+	configuration option "{0}". It was set to "{1}". All \
+	possible values for this setting are: {2}, or a comma separated list \
+	of the same.
 invalid-property-descriptors: Errors occurred while creating property \
 	descriptors for the following properties: {0}.
 invalid-property: The property named "{0}" was not recognized and will \

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/conf/TestAutoDetachProperty.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright 2006 The Apache Software Foundation.
+ *
+ * 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.apache.openjpa.conf;
+
+import java.util.*;
+
+import javax.persistence.*;
+
+import junit.framework.*;
+
+import org.apache.openjpa.kernel.AutoDetach;
+import org.apache.openjpa.persistence.*;
+
+public class TestAutoDetachProperty extends TestCase {
+    private EntityManager em;
+    private EntityManagerFactory emf;
+
+    protected void setUp() throws Exception {
+        // Don't modify system props, as we are trying to get as close as
+        // possible to testing props in persistence.xml
+        HashMap props = new HashMap(System.getProperties());
+        props.put("openjpa.AutoDetach", "commit,close,nontx-read");
+        emf = (OpenJPAEntityManagerFactory) Persistence
+                .createEntityManagerFactory("test", props);
+
+        em = emf.createEntityManager();
+    }
+
+    public void tearDown() throws Exception {
+        em.close();
+        em = null;
+    }
+
+    public void testIsAutoDetachingOnClose() {
+        assertTrue("not autodetaching on close as expected",
+                isAutoDetachingOnClose());
+    }
+
+    public void testIsAutoDetachingOnCommit() {
+        assertTrue("not autodetaching on commit as expected",
+                isAutoDetachingOnCommit());
+    }
+
+    public void testIsAutoDetachingOnNonTxRead() {
+        assertTrue("not autodetaching on nontransactional read as expected",
+                isAutoDetachingOnNonTxRead());
+    }
+
+    private boolean isAutoDetachingOnClose() {
+        int autoDetachFlags = OpenJPAPersistence.cast(em).getAutoDetach();
+        return (autoDetachFlags & AutoDetach.DETACH_CLOSE) > 0;
+    }
+
+    private boolean isAutoDetachingOnCommit() {
+        int autoDetachFlags = OpenJPAPersistence.cast(em).getAutoDetach();
+        return (autoDetachFlags & AutoDetach.DETACH_COMMIT) > 0;
+    }
+
+    private boolean isAutoDetachingOnNonTxRead() {
+        int autoDetachFlags = OpenJPAPersistence.cast(em).getAutoDetach();
+        return (autoDetachFlags & AutoDetach.DETACH_NONTXREAD) > 0;
+    }
+}

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/conf/TestBadAutoDetachProperty.java

@@ -0,0 +1,82 @@
+/*
+ * Copyright 2006 The Apache Software Foundation.
+ *
+ * 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.apache.openjpa.conf;
+
+import java.util.*;
+
+import javax.persistence.*;
+
+import junit.framework.*;
+
+import org.apache.openjpa.lib.util.ParseException;
+import org.apache.openjpa.persistence.*;
+import org.apache.openjpa.persistence.PersistenceException;
+
+public class TestBadAutoDetachProperty extends TestCase {
+    public void testEmptyValue() {
+        HashMap props = new HashMap(System.getProperties());
+        props.put("openjpa.AutoDetach", "");
+        EntityManagerFactory emf = (OpenJPAEntityManagerFactory) Persistence
+                .createEntityManagerFactory("test", props);
+        EntityManager em = emf.createEntityManager();
+        em.close();
+        emf.close();
+    }
+
+    public void testCommaOnlyValue() {
+        try {
+            HashMap props = new HashMap(System.getProperties());
+            props.put("openjpa.AutoDetach", ",");
+            EntityManagerFactory emf = (OpenJPAEntityManagerFactory) Persistence
+                    .createEntityManagerFactory("test", props);
+            EntityManager em = emf.createEntityManager();
+            em.close();
+            emf.close();
+        } catch (PersistenceException e) {
+            Throwable cause = e.getCause();
+            if (!(cause instanceof ParseException)) {
+                fail("Should have caught PersistenceException whose cause was "
+                        + "a ParseException. " + "Instead the cause was: "
+                        + cause);
+            }
+        } catch (RuntimeException e) {
+            fail("Should have caught a PersistenceException, instead caught: "
+                    + e);
+        }
+    }
+
+    public void testEmptyItemValue() {
+        try {
+            HashMap props = new HashMap(System.getProperties());
+            props.put("openjpa.AutoDetach", "close,,commit");
+            EntityManagerFactory emf = (OpenJPAEntityManagerFactory) Persistence
+                    .createEntityManagerFactory("test", props);
+            EntityManager em = emf.createEntityManager();
+            em.close();
+            emf.close();
+        } catch (PersistenceException e) {
+            Throwable cause = e.getCause();
+            if (!(cause instanceof ParseException)) {
+                fail("Should have caught PersistenceException whose cause was "
+                        + "a ParseException. " + "Instead the cause was: "
+                        + cause);
+            }
+        } catch (RuntimeException e) {
+            fail("Should have caught a PersistenceException, instead caught: "
+                    + e);
+        }
+    }
+}

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerFactoryImpl.java

@@ -186,10 +186,11 @@ public class EntityManagerFactoryImpl
 
         Broker broker = _factory.newBroker(user, pass, managed, retainMode,
             false);
-        // we should allow the user to specify these settings in conf
+            
-        // regardless of PersistenceContextType
+        // add autodetach for close and rollback conditions to the configuration
-        broker.setAutoDetach(AutoDetach.DETACH_CLOSE 
+        broker.setAutoDetach(AutoDetach.DETACH_CLOSE, true);
-            | AutoDetach.DETACH_ROLLBACK);
+        broker.setAutoDetach(AutoDetach.DETACH_ROLLBACK, true);
+        
         broker.setDetachedNew(false);
         OpenJPAEntityManager em = newEntityManagerImpl(broker);