From 4be5a5a194e541059709c9efcdd07c26990ab673 Mon Sep 17 00:00:00 2001 From: Pinaki Poddar Date: Fri, 12 Mar 2010 15:08:16 +0000 Subject: [PATCH] OPENJPA-1565: Raise correct time out exception git-svn-id: https://svn.apache.org/repos/asf/openjpa/trunk@922288 13f79535-47bb-0310-9956-ffa450edef68 --- .../jdbc/kernel/PessimisticLockManager.java | 6 +- .../openjpa/jdbc/sql/DB2Dictionary.java | 24 ++- .../apache/openjpa/jdbc/sql/DBDictionary.java | 79 ++++++---- .../openjpa/jdbc/sql/InformixDictionary.java | 26 ++-- .../openjpa/jdbc/sql/OracleDictionary.java | 28 ++-- .../jdbc/sql/sql-error-state-codes.xml | 8 +- .../org/apache/openjpa/kernel/Filters.java | 47 +++++- .../apache/openjpa/util/LockException.java | 12 +- .../apache/openjpa/util/OpenJPAException.java | 1 + .../apache/openjpa/util/StoreException.java | 1 + .../query/TestTimeoutException.java | 146 ++++++++++++++++++ .../lockmgr/TestPessimisticLocks.java | 13 +- .../persistence/LockTimeoutException.java | 5 +- .../persistence/PersistenceExceptions.java | 67 +++----- .../persistence/PessimisticLockException.java | 8 +- .../apache/openjpa/persistence/QueryImpl.java | 2 +- .../persistence/QueryTimeoutException.java | 8 +- 17 files changed, 323 insertions(+), 158 deletions(-) create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestTimeoutException.java diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PessimisticLockManager.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PessimisticLockManager.java index 1bcac2b21..222d79183 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PessimisticLockManager.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/PessimisticLockManager.java @@ -35,6 +35,7 @@ import org.apache.openjpa.kernel.OpenJPAStateManager; import org.apache.openjpa.kernel.StoreContext; import org.apache.openjpa.kernel.VersionLockManager; import org.apache.openjpa.lib.util.Localizer; +import org.apache.openjpa.util.Exceptions; import org.apache.openjpa.util.LockException; /** @@ -117,8 +118,7 @@ public class PessimisticLockManager JDBCFetchConfiguration fetch = _store.getFetchConfiguration(); if (dict.simulateLocking) return; - dict.assertSupport(dict.supportsSelectForUpdate, - "SupportsSelectForUpdate"); + dict.assertSupport(dict.supportsSelectForUpdate, "SupportsSelectForUpdate"); Object id = sm.getObjectId(); ClassMapping mapping = (ClassMapping) sm.getMetaData(); @@ -137,7 +137,7 @@ public class PessimisticLockManager checkLock(rs, sm, timeout); } } catch (SQLException se) { - throw SQLExceptions.getStore(se, dict, level); + throw SQLExceptions.getStore(se, Exceptions.toString(sm.getPersistenceCapable()), dict, level); } finally { if (stmnt != null) try { stmnt.close(); } catch (SQLException se) {} diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java index 66c3a121d..f705d3913 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DB2Dictionary.java @@ -923,22 +923,18 @@ public class DB2Dictionary } @Override - protected Boolean matchErrorState(int subtype, Set errorStates, - SQLException ex) { - Boolean recoverable = null; + protected boolean isFatalException(int subtype, SQLException ex) { String errorState = ex.getSQLState(); - if (errorStates.contains(errorState)) { - recoverable = Boolean.FALSE; - if (subtype == StoreException.LOCK && errorState.equals("57033") - && ex.getMessage().indexOf("80") != -1) { - recoverable = Boolean.TRUE; - } else if ((subtype == StoreException.QUERY && - errorState.equals("57014")) && - (ex.getErrorCode() == -952 || ex.getErrorCode() == -905)) { - recoverable = Boolean.TRUE; - } + int errorCode = ex.getErrorCode(); + if (subtype == StoreException.LOCK && "57033".equals(errorState) + && ex.getMessage().indexOf("80") != -1) { + return false; + } + if ((subtype == StoreException.QUERY && "57014".equals(errorState) && + (errorCode == -952 || errorCode == -905))) { + return false; } - return recoverable; + return super.isFatalException(subtype, ex); } @Override diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java index 890f9db0b..33837b535 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/DBDictionary.java @@ -110,6 +110,7 @@ import org.apache.openjpa.lib.util.Localizer.Message; import org.apache.openjpa.meta.FieldMetaData; import org.apache.openjpa.meta.JavaTypes; import org.apache.openjpa.meta.ValueStrategies; +import org.apache.openjpa.util.ExceptionInfo; import org.apache.openjpa.util.GeneralException; import org.apache.openjpa.util.InternalException; import org.apache.openjpa.util.InvalidStateException; @@ -4777,10 +4778,9 @@ public class DBDictionary * be determined by the implementation. This may take into account * DB-specific exception information in causes. */ - public OpenJPAException newStoreException(String msg, SQLException[] causes, - Object failed) { + public OpenJPAException newStoreException(String msg, SQLException[] causes, Object failed) { if (causes != null && causes.length > 0) { - OpenJPAException ret = narrow(msg, causes[0]); + OpenJPAException ret = narrow(msg, causes[0], failed); ret.setFailedObject(failed).setNestedThrowables(causes); return ret; } @@ -4789,36 +4789,29 @@ public class DBDictionary } /** - * Gets the subtype of StoreException by matching the given SQLException's + * Gets the category of StoreException by matching the given SQLException's * error state code to the list of error codes supplied by the dictionary. - * Returns -1 if no matching code can be found. + * + * @return a StoreException of {@link ExceptionInfo#GENERAL general} category + * if the given SQL Exception can not be further categorized. + * + * @see #matchErrorState(Map, SQLException) */ - OpenJPAException narrow(String msg, SQLException ex) { - Boolean recoverable = null; - int errorType = StoreException.GENERAL; - for (Integer type : sqlStateCodes.keySet()) { - Set errorStates = sqlStateCodes.get(type); - if (errorStates != null) { - recoverable = matchErrorState(type, errorStates, ex); - if (recoverable != null) { - errorType = type; - break; - } - } - } + OpenJPAException narrow(String msg, SQLException ex, Object failed) { + int errorType = matchErrorState(sqlStateCodes, ex); StoreException storeEx; switch (errorType) { case StoreException.LOCK: - storeEx = new LockException(msg); + storeEx = new LockException(failed); break; case StoreException.OBJECT_EXISTS: storeEx = new ObjectExistsException(msg); break; case StoreException.OBJECT_NOT_FOUND: - storeEx = new ObjectNotFoundException(msg); + storeEx = new ObjectNotFoundException(failed); break; case StoreException.OPTIMISTIC: - storeEx = new OptimisticException(msg); + storeEx = new OptimisticException(failed); break; case StoreException.REFERENTIAL_INTEGRITY: storeEx = new ReferentialIntegrityException(msg); @@ -4829,24 +4822,48 @@ public class DBDictionary default: storeEx = new StoreException(msg); } - if (recoverable != null) { - storeEx.setFatal(!recoverable); - } + storeEx.setFatal(isFatalException(errorType, ex)); return storeEx; } /** - * Determine if the SQLException argument matches any element in the - * errorStates. Dictionary subclass can override this method and extract + * Determine the more appropriate type of store exception by matching the SQL Error State of the + * the given SQLException to the given Error States categorized by error types. + * Dictionary subclass can override this method and extract * SQLException data to figure out if the exception is recoverable. * - * @return null if no match is found or a Boolean value indicates the - * exception is recoverable. + * @param errorStates classification of SQL error states by their specific nature. The keys of the + * map represent one of the constants defined in {@link StoreException}. The value corresponding to + * a key represent the set of SQL Error States representing specific category of database error. + * This supplied map is sourced from sql-error-state-codes.xml and filtered the + * error states for the current database. + * + * @param ex original SQL Exception as raised by the database driver. + * + * @return A constant indicating the category of error as defined in {@link StoreException}. */ - protected Boolean matchErrorState(int subtype, Set errorStates, - SQLException ex) { + protected int matchErrorState(Map> errorStates, SQLException ex) { String errorState = ex.getSQLState(); - return errorStates.contains(errorState) ? Boolean.FALSE : null; + for (Map.Entry> states : errorStates.entrySet()) { + if (states.getValue().contains(errorState)) + return states.getKey(); + } + return StoreException.GENERAL; + } + + /** + * Determine if the given SQL Exception is fatal or recoverable (such as a timeout). + * This implementation always returns true (i.e. all exceptions are fatal). + * The current dictionary implementation can overwrite this method to mark certain + * exception conditions as recoverable error. + + * @param subtype A constant indicating the category of error as defined in {@link StoreException}. + * @param ex original SQL Exception as raised by the database driver. + * + * @return false if the error is fatal. + */ + protected boolean isFatalException(int subtype, SQLException ex) { + return true; } /** diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/InformixDictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/InformixDictionary.java index e3ec49095..92eaa3867 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/InformixDictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/InformixDictionary.java @@ -377,23 +377,17 @@ public class InformixDictionary } @Override - protected Boolean matchErrorState(int subtype, Set errorStates, - SQLException ex) { - Boolean recoverable = null; - String errorState = ex.getSQLState(); - if (errorStates.contains(errorState)) { - // SQL State of IX000 is a general purpose Informix error code - // category, so only return Boolean.TRUE if we match SQL Codes - // recoverable = Boolean.FALSE; - if (subtype == StoreException.LOCK && - ex.getErrorCode() == -154) { - recoverable = Boolean.TRUE; - } else if (subtype == StoreException.QUERY && - ex.getErrorCode() == -213) { - recoverable = Boolean.TRUE; - } + protected boolean isFatalException(int subtype, SQLException ex) { + + // SQL State of IX000 is a general purpose Informix error code + // category, so only return Boolean.TRUE if we match SQL Codes + // recoverable = Boolean.FALSE; + if ((subtype == StoreException.LOCK && ex.getErrorCode() == -154) + ||(subtype == StoreException.QUERY && ex.getErrorCode() == -213)) { + return false; } - return recoverable; + + return super.isFatalException(subtype, ex); } } diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/OracleDictionary.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/OracleDictionary.java index ae6f16d68..4327fda15 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/OracleDictionary.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/sql/OracleDictionary.java @@ -1259,25 +1259,21 @@ public class OracleDictionary } @Override - protected Boolean matchErrorState(int subtype, Set errorStates, - SQLException ex) { - Boolean recoverable = null; + protected boolean isFatalException(int subtype, SQLException ex) { String errorState = ex.getSQLState(); int errorCode = ex.getErrorCode(); - if (errorStates.contains(errorState)) { - recoverable = Boolean.FALSE; - if ((subtype == StoreException.LOCK) - && ((errorState.equals("61000") && (errorCode == 54 || - errorCode == 60 || errorCode == 4020 || - errorCode == 4021 || errorCode == 4022)) - || (errorState.equals("42000") && errorCode == 2049))) { - recoverable = Boolean.TRUE; - } else if (subtype == StoreException.QUERY && - errorState.equals("72000") && errorCode == 1013) { - recoverable = Boolean.TRUE; - } + if ((subtype == StoreException.LOCK) + && (("61000".equals(errorState) && (errorCode == 54 || + errorCode == 60 || errorCode == 4020 || + errorCode == 4021 || errorCode == 4022)) + || ("42000".equals(errorState) && errorCode == 2049))) { + return false; + } + if (subtype == StoreException.QUERY && + "72000".equals(errorState) && errorCode == 1013) { + return false; } - return recoverable; + return super.isFatalException(subtype, ex); } @Override diff --git a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/sql/sql-error-state-codes.xml b/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/sql/sql-error-state-codes.xml index cae2617c9..72da6d2e8 100644 --- a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/sql/sql-error-state-codes.xml +++ b/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/sql/sql-error-state-codes.xml @@ -29,7 +29,7 @@ - 40001,57033,57011 + 40001,57033,57011,57014 23502,42912,23001,23504,23511,23512,23513,23515,23520 23505 @@ -155,12 +155,12 @@ - 1205,1213 + 41000 630,839,840,893,1062,1169,1215,1216,1217,1451,1452,1557 23000 - 41000 - + + 70100 diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Filters.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Filters.java index 5ef1737b2..e00aa4b2d 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Filters.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Filters.java @@ -245,11 +245,30 @@ public class Filters { return true; return false; } + + /** + * Convert the given value to match the given (presumably a setter) method argument type. + * + * @param o given value + * @param method a presumably setter method + * + * @return the same value if the method does not have one and only one input argument. + */ + public static Object convertToMatchMethodArgument(Object o, Method method) { + if (method == null || method.getParameterTypes().length != 1) { + return o; + } + return convert(o, method.getParameterTypes()[0], true); + } + public static Object convert(Object o, Class type) { + return convert(o, type, false); + } + /** * Convert the given value to the given type. */ - public static Object convert(Object o, Class type) { + public static Object convert(Object o, Class type, boolean strictNumericConversion) { if (o == null) return null; if (o.getClass() == type) @@ -315,13 +334,13 @@ public class Filters { throw new ClassCastException(_loc.get("cant-convert", o, o.getClass(), type).getMessage()); - if (type == Integer.class) { + if (type == Integer.class && allowNumericConversion(o.getClass(), type, strictNumericConversion)) { return ((Number) o).intValue(); - } else if (type == Float.class) { + } else if (type == Float.class && allowNumericConversion(o.getClass(), type, strictNumericConversion)) { return new Float(((Number) o).floatValue()); } else if (type == Double.class) { return new Double(((Number) o).doubleValue()); - } else if (type == Long.class) { + } else if (type == Long.class && allowNumericConversion(o.getClass(), type, strictNumericConversion)) { return ((Number) o).longValue(); } else if (type == BigDecimal.class) { // the BigDecimal constructor doesn't handle the @@ -339,14 +358,28 @@ public class Filters { return new BigDecimal(o.toString()); } else if (type == BigInteger.class) { return new BigInteger(o.toString()); - } else if (type == Short.class) { + } else if (type == Short.class && allowNumericConversion(o.getClass(), type, strictNumericConversion)) { return new Short(((Number) o).shortValue()); - } else if (type == Byte.class) { + } else if (type == Byte.class && allowNumericConversion(o.getClass(), type, strictNumericConversion)) { return new Byte(((Number) o).byteValue()); - } else { + } else if (!strictNumericConversion) { return ((Number) o).intValue(); + } else { + throw new ClassCastException(_loc.get("cant-convert", o, o.getClass(), type).getMessage()); } } + + private static boolean allowNumericConversion(Class actual, Class target, boolean strict) { + if (!strict || actual == target) + return true; + if (actual == Byte.class) return false; + if (actual == Double.class) return target == Float.class; + if (actual == Float.class) return target == Double.class; + if (actual == Integer.class) return target == Long.class || target == Short.class; + if (actual == Long.class) return target == Integer.class || target == Short.class; + if (actual == Short.class) return target == Long.class || target == Integer.class; + return false; + } /** * Add the given values. diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/util/LockException.java b/openjpa-kernel/src/main/java/org/apache/openjpa/util/LockException.java index ac79f0874..31d29d8bd 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/util/LockException.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/util/LockException.java @@ -30,13 +30,13 @@ import org.apache.openjpa.lib.util.Localizer; * @author Marc Prud'hommeaux * @since 0.3.1 */ +@SuppressWarnings("serial") public class LockException extends StoreException { - private static final transient Localizer _loc = Localizer.forPackage - (LockException.class); + private static final transient Localizer _loc = Localizer.forPackage(LockException.class); - private int timeout = -1; + private int timeout = -1; private int lockLevel = -1; public LockException(Object failed) { @@ -49,8 +49,7 @@ public class LockException } public LockException(Object failed, int timeout, int lockLevel) { - super(_loc.get("lock-timeout", Exceptions.toString(failed), - String.valueOf(timeout))); + super(_loc.get("lock-timeout", Exceptions.toString(failed), String.valueOf(timeout))); setFailedObject(failed); setTimeout(timeout); } @@ -86,8 +85,7 @@ public class LockException String str = super.toString(); if (timeout < 0) return str; - return str + Exceptions.SEP + "Timeout: " + timeout + ", LockLevel" - + lockLevel; + return str + Exceptions.SEP + "Timeout: " + timeout + ", LockLevel" + lockLevel; } private void writeObject(ObjectOutputStream out) diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/util/OpenJPAException.java b/openjpa-kernel/src/main/java/org/apache/openjpa/util/OpenJPAException.java index d50d40974..8cec3afeb 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/util/OpenJPAException.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/util/OpenJPAException.java @@ -35,6 +35,7 @@ import org.apache.openjpa.lib.util.Localizer.Message; * @author Abe White * @since 0.4.0 */ +@SuppressWarnings("serial") public abstract class OpenJPAException extends RuntimeException implements Serializable, ExceptionInfo { diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/util/StoreException.java b/openjpa-kernel/src/main/java/org/apache/openjpa/util/StoreException.java index 87a1a346a..8830df583 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/util/StoreException.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/util/StoreException.java @@ -26,6 +26,7 @@ import org.apache.openjpa.lib.util.Localizer.Message; * @author Marc Prud'hommeaux * @since 0.2.5 */ +@SuppressWarnings("serial") public class StoreException extends OpenJPAException { diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestTimeoutException.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestTimeoutException.java new file mode 100644 index 000000000..84b9a0e31 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestTimeoutException.java @@ -0,0 +1,146 @@ +/* + * 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.openjpa.persistence.query; + +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; + +import javax.persistence.EntityManager; +import javax.persistence.LockModeType; +import javax.persistence.LockTimeoutException; +import javax.persistence.Query; +import javax.persistence.QueryTimeoutException; + +import junit.framework.AssertionFailedError; + +import org.apache.openjpa.persistence.exception.PObject; +import org.apache.openjpa.persistence.test.SingleEMFTestCase; + +/** + * Tests that correct timeout exceptions are being thrown depending on whether it is a query or a lock operation. + * + * @author Pinaki Poddar + * + */ +public class TestTimeoutException extends SingleEMFTestCase { + private final Class entityClass = PObject.class; + private final ExecutorService scheduler = Executors.newCachedThreadPool(); + public void setUp() { + super.setUp(entityClass); + } + + public void testQueryTimeOutExceptionWhileQueryingWithLocksOnAlreadyLockedEntities() { + EntityManager em1 = emf.createEntityManager(); + EntityManager em2 = emf.createEntityManager(); + assertNotSame(em1, em2); + Object oid = createEntity(em1); + + em1.getTransaction().begin(); + Object entity = em1.find(entityClass, oid); + assertNotNull(entity); + em1.lock(entity, LockModeType.PESSIMISTIC_WRITE); + + em2.getTransaction().begin(); + final Query query = em2.createQuery("select p from PObject p"); + query.setLockMode(LockModeType.PESSIMISTIC_WRITE); + long timeout = 1000; + query.setHint("javax.persistence.query.timeout", timeout); + assertError(new Callable() { + public Throwable call() throws Exception { + try { + query.getResultList(); + } catch (Throwable t) { + return t; + } + return null; + } + }, QueryTimeoutException.class, timeout); + + assertTrue(em2.getTransaction().isActive()); + em2.getTransaction().rollback(); + em1.getTransaction().rollback(); + } + + public void testLockTimeOutExceptionWhileLockingAlreadyLockedEntities() { + EntityManager em1 = emf.createEntityManager(); + final EntityManager em2 = emf.createEntityManager(); + assertNotSame(em1, em2); + final Object oid = createEntity(em1); + + em1.getTransaction().begin(); + final Object entity1 = em1.find(entityClass, oid); + assertNotNull(entity1); + em1.lock(entity1, LockModeType.PESSIMISTIC_WRITE); + + em2.getTransaction().begin(); + final Object entity2 = em2.find(entityClass, oid); + final long timeout = 1000; + assertError(new Callable() { + public Throwable call() throws Exception { + try { + Map hint = new HashMap(); + hint.put("javax.persistence.lock.timeout", timeout); + em2.lock(entity2, LockModeType.PESSIMISTIC_WRITE, hint); + } catch (Throwable t) { + return t; + } + return null; + } + }, LockTimeoutException.class, timeout); + assertTrue(em2.getTransaction().isActive()); + em2.getTransaction().rollback(); + + em1.getTransaction().rollback(); + } + + + public Object createEntity(EntityManager em) { + long id = System.nanoTime(); + em.getTransaction().begin(); + PObject pc = new PObject(); + pc.setId(id); + em.persist(pc); + em.getTransaction().commit(); + return id; + } + + + /** + * Assert that an exception of proper type has been thrown by the given task within the given timeout. + * @param t + * @param expeceted + */ + void assertError(Callable task, Class expected, long timeout) { + try { + Future future = scheduler.submit(task); + Throwable error = future.get(); + if (error == null) { + throw new AssertionFailedError("No exception was raised but expected " + expected.getName()); + } else if (!expected.isAssignableFrom(error.getClass())) { + error.printStackTrace(); + throw new AssertionFailedError(error.getClass().getName() + " was raised but expected " + + expected.getName()); + } + } catch (Throwable t) { + t.printStackTrace(); + throw new AssertionFailedError(t.getClass().getName() + " was raised but expected " + + expected.getName()); + } + } + +} diff --git a/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/TestPessimisticLocks.java b/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/TestPessimisticLocks.java index f213d1917..571094bf5 100644 --- a/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/TestPessimisticLocks.java +++ b/openjpa-persistence-locking/src/test/java/org/apache/openjpa/persistence/lockmgr/TestPessimisticLocks.java @@ -30,6 +30,7 @@ import javax.persistence.QueryTimeoutException; import org.apache.openjpa.jdbc.conf.JDBCConfiguration; import org.apache.openjpa.jdbc.sql.DBDictionary; +import org.apache.openjpa.persistence.LockTimeoutException; import org.apache.openjpa.persistence.test.SQLListenerTestCase; /** @@ -40,6 +41,7 @@ public class TestPessimisticLocks extends SQLListenerTestCase { private DBDictionary dict = null; public void setUp() { + setUp(Employee.class, Department.class, "openjpa.LockManager", "mixed"); setSupportedDatabases( org.apache.openjpa.jdbc.sql.DerbyDictionary.class, // org.apache.openjpa.jdbc.sql.OracleDictionary.class, @@ -48,7 +50,6 @@ public class TestPessimisticLocks extends SQLListenerTestCase { return; } - setUp(Employee.class, Department.class, "openjpa.LockManager", "mixed"); String empTable = getMapping(Employee.class).getTable().getFullName(); String deptTable = getMapping(Department.class).getTable().getFullName(); @@ -125,10 +126,11 @@ public class TestPessimisticLocks extends SQLListenerTestCase { // find Employee(2) with a lock, should block and expected a PessimisticLockException em2.find(Employee.class, 2, LockModeType.PESSIMISTIC_READ, map); fail("Unexcpected find succeeded. Should throw a PessimisticLockException."); - } catch (QueryTimeoutException e) { + } catch (LockTimeoutException e) { // TODO: DB2: This is the current unexpected exception due to OPENJPA-991. // Remove this when the problem is fixed -// System.out.println("Caught " + e.getClass().getName() + ":" + e.getMessage()); + System.out.println("Caught " + e.getClass().getName() + ":" + e.getMessage() + " Failed " + + e.getFailedObject()); } catch (PessimisticLockException e) { // TODO: This is the expected exception but will be fixed under OPENJPA-991 // System.out.println("Caught " + e.getClass().getName() + ":" + e.getMessage()); @@ -208,10 +210,11 @@ public class TestPessimisticLocks extends SQLListenerTestCase { // find Employee(2) with a lock, should block and expected a PessimisticLockException em2.find(Employee.class, 2, LockModeType.PESSIMISTIC_READ, map); fail("Unexcpected find succeeded. Should throw a PessimisticLockException."); - } catch (QueryTimeoutException e) { + } catch (LockTimeoutException e) { // TODO: DB2: This is the current unexpected exception due to OPENJPA-991. // Remove this when the problem is fixed -// System.out.println("Caught " + e.getClass().getName() + ":" + e.getMessage()); + System.out.println("Caught " + e.getClass().getName() + ":" + e.getMessage() + " Failed " + + e.getFailedObject()); } catch (PessimisticLockException e) { // TODO: This is the expected exception but will be fixed under OPENJPA-991 // System.out.println("Caught " + e.getClass().getName() + ":" + e.getMessage()); diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/LockTimeoutException.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/LockTimeoutException.java index 9241f46bf..c9c08c804 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/LockTimeoutException.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/LockTimeoutException.java @@ -35,6 +35,7 @@ import org.apache.openjpa.util.StoreException; * @since 2.0.0 * @nojavadoc */ +@SuppressWarnings("serial") public class LockTimeoutException extends javax.persistence.LockTimeoutException implements Serializable, ExceptionInfo { @@ -43,12 +44,10 @@ public class LockTimeoutException private transient Object _failed = null; private transient Throwable[] _nested = null; - public LockTimeoutException(String msg, Throwable[] nested, - Object failed, boolean fatal) { + public LockTimeoutException(String msg, Throwable[] nested, Object failed) { super(msg); _nested = nested; _failed = failed; - _fatal = fatal; } public int getType() { diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistenceExceptions.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistenceExceptions.java index 637aedf09..4393c6a3c 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistenceExceptions.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistenceExceptions.java @@ -151,63 +151,40 @@ public class PersistenceExceptions */ private static Throwable translateStoreException(OpenJPAException ke) { Exception e; + int subtype = ke.getSubtype(); + String msg = ke.getMessage(); + Throwable[] nested = getNestedThrowables(ke); + Object failed = getFailedObject(ke); + boolean fatal = ke.isFatal(); Throwable cause = (ke.getNestedThrowables() != null && ke.getNestedThrowables().length == 1) ? ke.getNestedThrowables()[0] : null; - if (ke.getSubtype() == StoreException.OBJECT_NOT_FOUND - || cause instanceof ObjectNotFoundException) { - e = new org.apache.openjpa.persistence.EntityNotFoundException - (ke.getMessage(), getNestedThrowables(ke), - getFailedObject(ke), ke.isFatal()); - } else if (ke.getSubtype() == StoreException.OPTIMISTIC - || cause instanceof OptimisticException) { - e = new org.apache.openjpa.persistence.OptimisticLockException - (ke.getMessage(), getNestedThrowables(ke), - getFailedObject(ke), ke.isFatal()); - } else if (ke.getSubtype() == StoreException.LOCK - || cause instanceof LockException) { - LockException lockEx = (LockException) - (ke instanceof LockException ? ke : cause); - if (lockEx != null && lockEx.getLockLevel() >= - MixedLockLevels.LOCK_PESSIMISTIC_READ) { + if (subtype == StoreException.OBJECT_NOT_FOUND || cause instanceof ObjectNotFoundException) { + e = new org.apache.openjpa.persistence.EntityNotFoundException(msg, nested, failed, fatal); + } else if (subtype == StoreException.OPTIMISTIC || cause instanceof OptimisticException) { + e = new org.apache.openjpa.persistence.OptimisticLockException(msg, nested, failed, fatal); + } else if (subtype == StoreException.LOCK || cause instanceof LockException) { + LockException lockEx = (LockException) (ke instanceof LockException ? ke : cause); + if (lockEx != null && lockEx.getLockLevel() >= MixedLockLevels.LOCK_PESSIMISTIC_READ) { if (!lockEx.isFatal()) { - e = new org.apache.openjpa.persistence - .LockTimeoutException( - ke.getMessage(), getNestedThrowables(ke), - getFailedObject(ke), ke.isFatal()); + e = new org.apache.openjpa.persistence.LockTimeoutException(msg, nested, failed); } else { - e = new org.apache.openjpa.persistence - .PessimisticLockException( - ke.getMessage(), getNestedThrowables(ke), - getFailedObject(ke), ke.isFatal()); + e = new org.apache.openjpa.persistence.PessimisticLockException(msg, nested, failed); } } else { - e = new org.apache.openjpa.persistence.OptimisticLockException( - ke.getMessage(), getNestedThrowables(ke), - getFailedObject(ke), ke.isFatal()); + e = new org.apache.openjpa.persistence.OptimisticLockException(msg, nested, failed, fatal); } - } else if (ke.getSubtype() == StoreException.OBJECT_EXISTS - || cause instanceof ObjectExistsException) { - e = new org.apache.openjpa.persistence.EntityExistsException - (ke.getMessage(), getNestedThrowables(ke), - getFailedObject(ke), ke.isFatal()); - } else if (ke.getSubtype() == StoreException.QUERY - || cause instanceof QueryException) { - QueryException queryEx = (QueryException) - (ke instanceof QueryException ? ke : cause); + } else if (subtype == StoreException.OBJECT_EXISTS || cause instanceof ObjectExistsException) { + e = new org.apache.openjpa.persistence.EntityExistsException(msg, nested, failed, fatal); + } else if (subtype == StoreException.QUERY || cause instanceof QueryException) { + QueryException queryEx = (QueryException) (ke instanceof QueryException ? ke : cause); if (!queryEx.isFatal()) { - e = new org.apache.openjpa.persistence.QueryTimeoutException( - ke.getMessage(), getNestedThrowables(ke), - getFailedObject(ke), ke.isFatal()); + e = new org.apache.openjpa.persistence.QueryTimeoutException(msg, nested, failed, false); } else { - e = new org.apache.openjpa.persistence.PersistenceException( - ke.getMessage(), getNestedThrowables(ke), - getFailedObject(ke), ke.isFatal()); + e = new org.apache.openjpa.persistence.PersistenceException(msg, nested, failed, true); } } else { - e = new org.apache.openjpa.persistence.PersistenceException - (ke.getMessage(), getNestedThrowables(ke), - getFailedObject(ke), ke.isFatal()); + e = new org.apache.openjpa.persistence.PersistenceException(msg, nested, failed, fatal); } e.setStackTrace(ke.getStackTrace()); return e; diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PessimisticLockException.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PessimisticLockException.java index 2ec4bd315..278e466cf 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PessimisticLockException.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PessimisticLockException.java @@ -31,24 +31,24 @@ import org.apache.openjpa.util.StoreException; /** * Pessimistic concurrency violation. + * This exception is always fatal in contrast to {@linkplain LockTimeoutException}. * * @since 2.0.0 * @nojavadoc */ +@SuppressWarnings("serial") public class PessimisticLockException extends javax.persistence.PessimisticLockException implements Serializable, ExceptionInfo { - private transient boolean _fatal = false; + private transient boolean _fatal = true; private transient Object _failed = null; private transient Throwable[] _nested = null; - public PessimisticLockException(String msg, Throwable[] nested, - Object failed, boolean fatal) { + public PessimisticLockException(String msg, Throwable[] nested, Object failed) { super(msg); _nested = nested; _failed = failed; - _fatal = fatal; } public int getType() { diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java index 7568efeeb..a52fac074 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryImpl.java @@ -293,7 +293,7 @@ public class QueryImpl implements OpenJPAQuerySPI, Serializable { } return result; } catch (LockTimeoutException e) { - throw new QueryTimeoutException(e.getMessage(), new Throwable[]{e}, getQueryString(), e.isFatal()); + throw new QueryTimeoutException(e.getMessage(), new Throwable[]{e}, this); } finally { unlock(); } diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryTimeoutException.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryTimeoutException.java index 6bdd848ff..5e55615a1 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryTimeoutException.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/QueryTimeoutException.java @@ -35,6 +35,7 @@ import org.apache.openjpa.util.StoreException; * @since 2.0.0 * @nojavadoc */ +@SuppressWarnings("serial") public class QueryTimeoutException extends javax.persistence.QueryTimeoutException implements Serializable, ExceptionInfo { @@ -43,8 +44,11 @@ public class QueryTimeoutException private transient Object _failed = null; private transient Throwable[] _nested = null; - public QueryTimeoutException(String msg, Throwable[] nested, - Object failed, boolean fatal) { + public QueryTimeoutException(String msg, Throwable[] nested, Object failed) { + this(msg, nested, failed, false); + } + + public QueryTimeoutException(String msg, Throwable[] nested, Object failed, boolean fatal) { super(msg); _nested = nested; _failed = failed;