[java-opensaml] branch master updated: OSJ-269 close entity manager
Daniel Fisher
dfisher at vt.edu
Sun Apr 14 22:19:55 EDT 2019
This is an automated email from the git hooks/post-receive script.
dfisher pushed a commit to branch master
in repository java-opensaml.
View the commit online:
http://git.shibboleth.net/view/?p=java-opensaml.git;a=commit;h=880775b5195f5fb7d16f06c9f6fdfee925ff340c
The following commit(s) were added to refs/heads/master by this push:
new 880775b OSJ-269 close entity manager
880775b is described below
commit 880775b5195f5fb7d16f06c9f6fdfee925ff340c
Author: Daniel Fisher <dfisher at vt.edu>
AuthorDate: Sun Apr 7 19:53:27 2019 -0400
OSJ-269 close entity manager
Close entity manager before retries.
Consolidate common code into methods.
Update JPA test to use pooling.
---
opensaml-storage-impl/pom.xml | 2 +-
.../opensaml/storage/impl/JPAStorageService.java | 339 ++++++---------------
.../opensaml/storage/impl/jpa-spring-context.xml | 12 +-
3 files changed, 98 insertions(+), 255 deletions(-)
diff --git a/opensaml-storage-impl/pom.xml b/opensaml-storage-impl/pom.xml
index cecbc76..69e3ba7 100644
--- a/opensaml-storage-impl/pom.xml
+++ b/opensaml-storage-impl/pom.xml
@@ -118,7 +118,7 @@
<dependency>
<groupId>mysql</groupId>
<artifactId>mysql-connector-java</artifactId>
- <version>5.1.34</version>
+ <version>8.0.15</version>
<scope>test</scope>
</dependency>
diff --git a/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/JPAStorageService.java b/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/JPAStorageService.java
index b0b68e1..35c0533 100644
--- a/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/JPAStorageService.java
+++ b/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/JPAStorageService.java
@@ -124,7 +124,7 @@ public class JPAStorageService extends AbstractStorageService implements Storage
EntityManager manager = null;
try {
int retry = -1;
- RollbackException lastThrown = null;
+ RollbackException lastThrown;
do {
EntityTransaction transaction = null;
try {
@@ -158,55 +158,25 @@ public class JPAStorageService extends AbstractStorageService implements Storage
expiration,});
return true;
} catch (final EntityExistsException e) {
- if (transaction != null && transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
+ rollbackTransaction(transaction);
log.debug("Duplicate record '{}' in context '{}' with expiration '{}'", key, context, expiration);
return false;
} catch (final RollbackException e) {
lastThrown = e;
retry++;
} catch (final Exception e) {
- if (transaction != null && transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
+ rollbackTransaction(transaction);
log.error("Error creating record '{}' in context '{}' with expiration '{}'", key, context,
expiration, e);
throw new IOException(e);
} finally {
- if (transaction != null && transaction.isActive() && !transaction.getRollbackOnly()) {
- try {
- transaction.commit();
- } catch (final Exception e) {
- log.error("Error committing transaction", e);
- if (transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
- }
- }
+ commitTransaction(transaction);
+ closeEntityManager(manager);
}
} while (retry < transactionRetry);
throw lastThrown;
} finally {
- if (manager != null && manager.isOpen()) {
- try {
- manager.close();
- } catch (final Exception e) {
- log.error("Error closing entity manager", e);
- }
- }
+ closeEntityManager(manager);
}
}
@@ -226,13 +196,7 @@ public class JPAStorageService extends AbstractStorageService implements Storage
return executeNamedQuery(manager, "JPAStorageRecord.findAll", null, StorageRecord.class,
LockModeType.PESSIMISTIC_READ);
} finally {
- if (manager != null && manager.isOpen()) {
- try {
- manager.close();
- } catch (final Exception e) {
- log.error("Error closing entity manager", e);
- }
- }
+ closeEntityManager(manager);
}
}
@@ -254,13 +218,7 @@ public class JPAStorageService extends AbstractStorageService implements Storage
return executeNamedQuery(manager, "JPAStorageRecord.findByContext", params, StorageRecord.class,
LockModeType.PESSIMISTIC_READ);
} finally {
- if (manager != null && manager.isOpen()) {
- try {
- manager.close();
- } catch (final Exception e) {
- log.error("Error closing entity manager", e);
- }
- }
+ closeEntityManager(manager);
}
}
@@ -278,13 +236,7 @@ public class JPAStorageService extends AbstractStorageService implements Storage
return executeNamedQuery(manager, "JPAStorageRecord.findAllContexts", null, String.class,
LockModeType.OPTIMISTIC);
} finally {
- if (manager != null && manager.isOpen()) {
- try {
- manager.close();
- } catch (final Exception e) {
- log.error("Error closing entity manager", e);
- }
- }
+ closeEntityManager(manager);
}
}
@@ -340,36 +292,11 @@ public class JPAStorageService extends AbstractStorageService implements Storage
return new Pair<Long, StorageRecord>(entity.getVersion(), entity);
} catch (final Exception e) {
log.error("Error reading record '{}' in context '{}'", key, context, e);
- if (transaction != null && transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
+ rollbackTransaction(transaction);
throw new IOException(e);
} finally {
- if (transaction != null && transaction.isActive() && !transaction.getRollbackOnly()) {
- try {
- transaction.commit();
- } catch (final Exception e) {
- log.error("Error committing transaction", e);
- if (transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
- }
- }
- if (manager != null && manager.isOpen()) {
- try {
- manager.close();
- } catch (final Exception e) {
- log.error("Error closing entity manager", e);
- }
- }
+ commitTransaction(transaction);
+ closeEntityManager(manager);
}
}
@@ -424,7 +351,7 @@ public class JPAStorageService extends AbstractStorageService implements Storage
EntityManager manager = null;
try {
int retry = -1;
- RollbackException lastThrown = null;
+ RollbackException lastThrown;
do {
EntityTransaction transaction = null;
try {
@@ -467,40 +394,16 @@ public class JPAStorageService extends AbstractStorageService implements Storage
retry++;
} catch (final Exception e) {
log.error("Error updating record '{}' in context '{}'", key, context, e);
- if (transaction != null && transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
+ rollbackTransaction(transaction);
throw new IOException(e);
} finally {
- if (transaction != null && transaction.isActive() && !transaction.getRollbackOnly()) {
- try {
- transaction.commit();
- } catch (final Exception e) {
- log.error("Error committing transaction", e);
- if (transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
- }
- }
+ commitTransaction(transaction);
+ closeEntityManager(manager);
}
} while (retry < transactionRetry);
throw lastThrown;
} finally {
- if (manager != null && manager.isOpen()) {
- try {
- manager.close();
- } catch (final Exception e) {
- log.error("Error closing entity manager", e);
- }
- }
+ closeEntityManager(manager);
}
}
@@ -540,7 +443,7 @@ public class JPAStorageService extends AbstractStorageService implements Storage
EntityManager manager = null;
try {
int retry = -1;
- RollbackException lastThrown = null;
+ RollbackException lastThrown;
do {
EntityTransaction transaction = null;
try {
@@ -568,40 +471,16 @@ public class JPAStorageService extends AbstractStorageService implements Storage
retry++;
} catch (final Exception e) {
log.error("Error deleting record '{}' in context '{}'", key, context, e);
- if (transaction != null && transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
+ rollbackTransaction(transaction);
throw new IOException(e);
} finally {
- if (transaction != null && transaction.isActive() && !transaction.getRollbackOnly()) {
- try {
- transaction.commit();
- } catch (final Exception e) {
- log.error("Error committing transaction", e);
- if (transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
- }
- }
+ commitTransaction(transaction);
+ closeEntityManager(manager);
}
} while (retry < transactionRetry);
throw lastThrown;
} finally {
- if (manager != null && manager.isOpen()) {
- try {
- manager.close();
- } catch (final Exception e) {
- log.error("Error closing entity manager", e);
- }
- }
+ closeEntityManager(manager);
}
}
@@ -614,7 +493,7 @@ public class JPAStorageService extends AbstractStorageService implements Storage
EntityManager manager = null;
try {
int retry = -1;
- RollbackException lastThrown = null;
+ RollbackException lastThrown;
do {
EntityTransaction transaction = null;
try {
@@ -635,40 +514,16 @@ public class JPAStorageService extends AbstractStorageService implements Storage
retry++;
} catch (final Exception e) {
log.error("Error updating context expiration in context '{}'", context, e);
- if (transaction != null && transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
+ rollbackTransaction(transaction);
throw new IOException(e);
} finally {
- if (transaction != null && transaction.isActive() && !transaction.getRollbackOnly()) {
- try {
- transaction.commit();
- } catch (final Exception e) {
- log.error("Error committing transaction", e);
- if (transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
- }
- }
+ commitTransaction(transaction);
+ closeEntityManager(manager);
}
} while (retry < transactionRetry);
throw lastThrown;
} finally {
- if (manager != null && manager.isOpen()) {
- try {
- manager.close();
- } catch (final Exception e) {
- log.error("Error closing entity manager", e);
- }
- }
+ closeEntityManager(manager);
}
}
@@ -701,7 +556,7 @@ public class JPAStorageService extends AbstractStorageService implements Storage
EntityManager manager = null;
try {
int retry = -1;
- RollbackException lastThrown = null;
+ RollbackException lastThrown;
do {
EntityTransaction transaction = null;
try {
@@ -726,40 +581,16 @@ public class JPAStorageService extends AbstractStorageService implements Storage
retry++;
} catch (final Exception e) {
log.error("Error deleting context '{}'", context, e);
- if (transaction != null && transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
+ rollbackTransaction(transaction);
throw new IOException(e);
} finally {
- if (transaction != null && transaction.isActive() && !transaction.getRollbackOnly()) {
- try {
- transaction.commit();
- } catch (final Exception e) {
- log.error("Error committing transaction", e);
- if (transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
- }
- }
+ commitTransaction(transaction);
+ closeEntityManager(manager);
}
} while (retry < transactionRetry);
throw lastThrown;
} finally {
- if (manager != null && manager.isOpen()) {
- try {
- manager.close();
- } catch (final Exception e) {
- log.error("Error closing entity manager", e);
- }
- }
+ closeEntityManager(manager);
}
}
@@ -777,7 +608,7 @@ public class JPAStorageService extends AbstractStorageService implements Storage
EntityManager manager = null;
try {
int retry = -1;
- RollbackException lastThrown = null;
+ RollbackException lastThrown;
do {
EntityTransaction transaction = null;
try {
@@ -796,40 +627,16 @@ public class JPAStorageService extends AbstractStorageService implements Storage
retry++;
} catch (final Exception e) {
log.error("Error deleting with expiration '{}'", expiration, e);
- if (transaction != null && transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
+ rollbackTransaction(transaction);
throw new IOException(e);
} finally {
- if (transaction != null && transaction.isActive() && !transaction.getRollbackOnly()) {
- try {
- transaction.commit();
- } catch (final Exception e) {
- log.error("Error committing transaction", e);
- if (transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
- }
- }
+ commitTransaction(transaction);
+ closeEntityManager(manager);
}
} while (retry < transactionRetry);
throw lastThrown;
} finally {
- if (manager != null && manager.isOpen()) {
- try {
- manager.close();
- } catch (final Exception e) {
- log.error("Error closing entity manager", e);
- }
- }
+ closeEntityManager(manager);
}
}
@@ -867,29 +674,10 @@ public class JPAStorageService extends AbstractStorageService implements Storage
results.addAll(queryResults.getResultList());
} catch (final Exception e) {
log.error("Error executing named query", e);
- if (transaction != null && transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
+ rollbackTransaction(transaction);
throw new IOException(e);
} finally {
- if (transaction != null && transaction.isActive() && !transaction.getRollbackOnly()) {
- try {
- transaction.commit();
- } catch (final Exception e) {
- log.error("Error committing transaction", e);
- if (transaction.isActive()) {
- try {
- transaction.rollback();
- } catch (final Exception ex) {
- log.error("Error rolling back transaction", e);
- }
- }
- }
- }
+ commitTransaction(transaction);
}
return results;
}
@@ -913,4 +701,53 @@ public class JPAStorageService extends AbstractStorageService implements Storage
}
};
}
+
+ /**
+ * Commits the supplied transaction if {@link EntityTransaction#isActive()} and not {@link
+ * EntityTransaction#getRollbackOnly()}. Logs any exception that occurs.
+ *
+ * @param transaction to commit
+ */
+ private void commitTransaction(@Nullable final EntityTransaction transaction)
+ {
+ if (transaction != null && transaction.isActive() && !transaction.getRollbackOnly()) {
+ try {
+ transaction.commit();
+ } catch (final Exception e) {
+ log.error("Error committing transaction", e);
+ }
+ }
+ }
+
+ /**
+ * Rolls back the supplied transaction if {@link EntityTransaction#isActive()}. Logs any exception that occurs.
+ *
+ * @param transaction to roll back
+ */
+ private void rollbackTransaction(@Nullable final EntityTransaction transaction)
+ {
+ if (transaction != null && transaction.isActive()) {
+ try {
+ transaction.rollback();
+ } catch (final Exception e) {
+ log.error("Error rolling back transaction", e);
+ }
+ }
+ }
+
+ /**
+ * Closes the supplied entity manager if {@link EntityManager#isOpen()}. Logs any exception that occurs.
+ *
+ * @param manager to close
+ */
+ private void closeEntityManager(@Nullable final EntityManager manager)
+ {
+ if (manager != null && manager.isOpen()) {
+ try {
+ manager.close();
+ } catch (final Exception e) {
+ log.error("Error closing entity manager", e);
+ }
+ }
+ }
}
diff --git a/opensaml-storage-impl/src/test/resources/org/opensaml/storage/impl/jpa-spring-context.xml b/opensaml-storage-impl/src/test/resources/org/opensaml/storage/impl/jpa-spring-context.xml
index 6f4e7e6..c8302fd 100644
--- a/opensaml-storage-impl/src/test/resources/org/opensaml/storage/impl/jpa-spring-context.xml
+++ b/opensaml-storage-impl/src/test/resources/org/opensaml/storage/impl/jpa-spring-context.xml
@@ -54,9 +54,15 @@
</bean>
<bean id="mysqlDataSource" class="org.apache.commons.dbcp2.BasicDataSource" destroy-method="close" lazy-init="true"
p:driverClassName="com.mysql.jdbc.Driver"
- p:url="jdbc:mysql://localhost:3306/storageservice"
+ p:url="jdbc:mysql://localhost:3306/storageservice?serverTimezone=UTC"
p:username="shib"
- p:password="p at ssw0rd" />
+ p:password="p at ssw0rd"
+ p:maxTotal="1"
+ p:maxIdle="1"
+ p:maxWaitMillis="15000"
+ p:testOnBorrow="true"
+ p:validationQuery="select 1"
+ p:validationQueryTimeout="5" />
<!-- Oracle configuration
@@ -79,4 +85,4 @@
p:username="pmuser"
p:password="oracle" />
-</beans>
\ No newline at end of file
+</beans>
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list