From 2ac1bcccbb4070da2c86bfee4c7368d9466141ca Mon Sep 17 00:00:00 2001 From: Marc Prud'hommeaux Date: Wed, 31 Jan 2007 22:27:29 +0000 Subject: [PATCH] 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 --- .../apache/openjpa/conf/AutoDetachValue.java | 16 +++- .../openjpa/lib/conf/StringListValue.java | 73 ++++++++++++++++- .../openjpa/lib/conf/localizer.properties | 4 + .../openjpa/conf/TestAutoDetachProperty.java | 76 +++++++++++++++++ .../conf/TestBadAutoDetachProperty.java | 82 +++++++++++++++++++ .../persistence/EntityManagerFactoryImpl.java | 9 +- 6 files changed, 253 insertions(+), 7 deletions(-) create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/conf/TestAutoDetachProperty.java create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/conf/TestBadAutoDetachProperty.java diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/conf/AutoDetachValue.java b/openjpa-kernel/src/main/java/org/apache/openjpa/conf/AutoDetachValue.java index 6ab824bed..20c0c87d2 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/conf/AutoDetachValue.java +++ b/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,13 +39,13 @@ 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", }; - + private int _flags; private boolean _flagsSet; @@ -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; + } } diff --git a/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/StringListValue.java b/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/StringListValue.java index 066bb4470..6e3e7ed86 100644 --- a/openjpa-lib/src/main/java/org/apache/openjpa/lib/conf/StringListValue.java +++ b/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; @@ -50,6 +59,61 @@ public class StringListValue extends Value { public Class getValueType() { 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, ", "); @@ -57,14 +121,21 @@ public class StringListValue extends Value { 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()); + } + } diff --git a/openjpa-lib/src/main/resources/org/apache/openjpa/lib/conf/localizer.properties b/openjpa-lib/src/main/resources/org/apache/openjpa/lib/conf/localizer.properties index 5c0c622d7..3c4a9bccb 100644 --- a/openjpa-lib/src/main/resources/org/apache/openjpa/lib/conf/localizer.properties +++ b/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 \ diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/conf/TestAutoDetachProperty.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/conf/TestAutoDetachProperty.java new file mode 100644 index 000000000..90d769e7a --- /dev/null +++ b/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; + } +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/conf/TestBadAutoDetachProperty.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/conf/TestBadAutoDetachProperty.java new file mode 100644 index 000000000..be119a814 --- /dev/null +++ b/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); + } + } +} diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerFactoryImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerFactoryImpl.java index d06b017b7..8002d043d 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/EntityManagerFactoryImpl.java +++ b/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 - broker.setAutoDetach(AutoDetach.DETACH_CLOSE - | AutoDetach.DETACH_ROLLBACK); + + // add autodetach for close and rollback conditions to the configuration + broker.setAutoDetach(AutoDetach.DETACH_CLOSE, true); + broker.setAutoDetach(AutoDetach.DETACH_ROLLBACK, true); + broker.setDetachedNew(false); OpenJPAEntityManager em = newEntityManagerImpl(broker);