[java-opensaml COMMIT] in /trunk/opensaml-storage-impl/src: main/java/org/opensaml/storage/impl/JPAStorageService.jav...

[noreply at shibboleth.net]

Author: dfisher
Date: Wed Feb 11 09:14:56 2015
New Revision: 4219

URL: http://svn.shibboleth.net/view/java-opensaml?rev=4219&view=rev
Log:
OSJ-101

Fix implementation of create to honor API contract.
Refactor all operations to use pessimistic read/write.
(Notable exception for reading all context values)
Catch and log any exceptions related to transaction and entity manager cleanup.

Modified:
    trunk/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/JPAStorageService.java
    trunk/opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/JPAStorageServiceTest.java
    trunk/opensaml-storage-impl/src/test/resources/org/opensaml/storage/impl/jpa-spring-context.xml

Modified: trunk/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/JPAStorageService.java
URL: http://svn.shibboleth.net/view/java-opensaml/trunk/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/JPAStorageService.java?rev=4219&r1=4218&r2=4219&view=diff
==============================================================================
--- trunk/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/JPAStorageService.java (original)
+++ trunk/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/JPAStorageService.java Wed Feb 11 09:14:56 2015
@@ -26,9 +26,11 @@
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+import javax.persistence.EntityExistsException;
 import javax.persistence.EntityManager;
 import javax.persistence.EntityManagerFactory;
 import javax.persistence.EntityTransaction;
+import javax.persistence.LockModeType;
 import javax.persistence.Query;
 
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
@@ -64,7 +66,7 @@
 
         setContextSize(JPAStorageRecord.CONTEXT_SIZE);
         setKeySize(JPAStorageRecord.KEY_SIZE);
-        setValueSize(JPAStorageRecord.VALUE_SIZE);
+        setValueSize(Integer.MAX_VALUE);
     }
 
     /** {@inheritDoc} */
@@ -75,6 +77,7 @@
         super.doDestroy();
     }
 
+    // Checkstyle: CyclomaticComplexity OFF
     /** {@inheritDoc} */
     @Override public boolean create(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String key,
             @Nonnull @NotEmpty final String value, @Nullable @Positive final Long expiration) throws IOException {
@@ -82,43 +85,72 @@
         EntityTransaction transaction = null;
         try {
             manager = entityManagerFactory.createEntityManager();
-            JPAStorageRecord entity = manager.find(JPAStorageRecord.class, new JPAStorageRecord.RecordId(context, key));
+            transaction = manager.getTransaction();
+            transaction.begin();
+            JPAStorageRecord entity =
+                    manager.find(JPAStorageRecord.class, new JPAStorageRecord.RecordId(context, key),
+                            LockModeType.PESSIMISTIC_WRITE);
             if (entity != null) {
                 // Not yet expired?
                 final Long exp = entity.getExpiration();
                 if (exp == null || System.currentTimeMillis() < exp) {
+                    log.debug("Duplicate record '{}' in context '{}' with expiration '{}'", key, context, expiration);
                     return false;
                 }
 
-                // It's dead, so we can just delete it.
-                delete(context, key);
-            }
-
-            transaction = manager.getTransaction();
-            transaction.begin();
-            entity = new JPAStorageRecord();
-            entity.setContext(context);
-            entity.setKey(key);
+                // It's dead, reset the version for merge.
+                entity.resetVersion();
+            } else {
+                entity = new JPAStorageRecord();
+                entity.setContext(context);
+                entity.setKey(key);
+            }
+
             entity.setValue(value);
             entity.setExpiration(expiration);
-            manager.persist(entity);
-            transaction.commit();
-
-            log.debug("Inserted record '{}' in context '{}' with expiration '{}'", new Object[] {key, context,
+            manager.merge(entity);
+            log.debug("Merged record '{}' in context '{}' with expiration '{}'", new Object[] {key, context,
                     expiration,});
             return true;
+        } catch (final EntityExistsException e) {
+            if (transaction != null && transaction.isActive()) {
+                try {
+                    transaction.rollback();
+                } catch (Exception ex) {
+                    log.error("Error rolling back transaction", e);
+                }
+            }
+            log.debug("Duplicate record '{}' in context '{}' with expiration '{}'", key, context, expiration);
+            return false;
         } catch (final Exception e) {
+            if (transaction != null && transaction.isActive()) {
+                try {
+                    transaction.rollback();
+                } catch (Exception ex) {
+                    log.error("Error rolling back transaction", e);
+                }
+            }

[... 731 lines stripped ...]