From 4087608e1a57402ddd23736b623c1149a2cd409d Mon Sep 17 00:00:00 2001 From: Robert von Burg Date: Thu, 5 Oct 2017 09:54:42 +0200 Subject: [PATCH] [Fix] Throw Exception on programmer mistake if no commitOnClose with + --- .../persistence/api/AbstractTransaction.java | 17 +++++----- .../test/java/li/strolch/service/TxTest.java | 5 +-- .../li/strolch/xmlpers/api/ObjectDao.java | 34 +++++++++---------- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/li.strolch.agent/src/main/java/li/strolch/persistence/api/AbstractTransaction.java b/li.strolch.agent/src/main/java/li/strolch/persistence/api/AbstractTransaction.java index 421ce707b..297f97dbf 100644 --- a/li.strolch.agent/src/main/java/li/strolch/persistence/api/AbstractTransaction.java +++ b/li.strolch.agent/src/main/java/li/strolch/persistence/api/AbstractTransaction.java @@ -918,7 +918,7 @@ public abstract class AbstractTransaction implements StrolchTransaction { exc); //$NON-NLS-1$ } logger.error("Transaction failed due to " + e.getMessage(), e); - handleFailure(start, ex); + handleFailure(false, start, ex); } try { @@ -927,14 +927,12 @@ public abstract class AbstractTransaction implements StrolchTransaction { } catch (Exception ex) { logger.error("Transaction failed due to " + e.getMessage(), e); logger.error("Failed to commit transaction and then rollback due to " + ex.getMessage(), ex); - handleFailure(start, e); + handleFailure(false, start, e); } this.txResult.setState(TransactionState.FAILED); - String msg = "Strolch Transaction for realm {0} failed due to {1}"; //$NON-NLS-1$ - msg = MessageFormat.format(msg, getRealmName(), ExceptionHelper.getExceptionMessage(e)); - throw new StrolchTransactionException(msg, e); + handleFailure(true, start, e); } finally { releaseElementLocks(); @@ -952,7 +950,7 @@ public abstract class AbstractTransaction implements StrolchTransaction { handleRollback(start); this.txResult.setState(TransactionState.ROLLED_BACK); } catch (Exception e) { - handleFailure(start, e); + handleFailure(true, start, e); this.txResult.setState(TransactionState.FAILED); } finally { releaseElementLocks(); @@ -962,6 +960,7 @@ public abstract class AbstractTransaction implements StrolchTransaction { @Override public void autoCloseableReadOnly() throws StrolchTransactionException { long start = System.nanoTime(); + boolean throwException = false; try { this.txResult.setState(TransactionState.CLOSING); @@ -995,7 +994,7 @@ public abstract class AbstractTransaction implements StrolchTransaction { } catch (Exception e1) { logger.error("Failed to rollback after exception", e); } - handleFailure(start, e); + handleFailure(true, start, e); this.txResult.setState(TransactionState.FAILED); } finally { releaseElementLocks(); @@ -1114,7 +1113,7 @@ public abstract class AbstractTransaction implements StrolchTransaction { logger.error(sb.toString()); } - private void handleFailure(long closeStartNanos, Exception e) { + private void handleFailure(boolean throwEx, long closeStartNanos, Exception e) { long end = System.nanoTime(); long txDuration = end - this.txResult.getStartNanos(); @@ -1153,7 +1152,7 @@ public abstract class AbstractTransaction implements StrolchTransaction { msg = MessageFormat.format(msg, getRealmName(), ExceptionHelper.getExceptionMessage(e), sb.toString()); StrolchTransactionException ex = new StrolchTransactionException(msg, e); - if (this.closeStrategy == TransactionCloseStrategy.COMMIT) + if (throwEx) throw ex; else logger.error("Transaction failed in non-commit mode! Logging TX exception, so exception is not suppressed.", diff --git a/li.strolch.service/src/test/java/li/strolch/service/TxTest.java b/li.strolch.service/src/test/java/li/strolch/service/TxTest.java index 22f7c2dfd..3a4791e90 100644 --- a/li.strolch.service/src/test/java/li/strolch/service/TxTest.java +++ b/li.strolch.service/src/test/java/li/strolch/service/TxTest.java @@ -18,6 +18,7 @@ package li.strolch.service; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import li.strolch.agent.api.StrolchAgent; import li.strolch.model.ModelGenerator; import li.strolch.model.Order; import li.strolch.model.Resource; @@ -190,7 +191,7 @@ public class TxTest extends AbstractRealmServiceTest { @Override protected ServiceResult internalDoService(ServiceArgument arg) throws Exception { - String id = "flushSuccessfully"; + String id = StrolchAgent.getUniqueId(); Resource resource = ModelGenerator.createResource(id, id, id); boolean txFailed = false; @@ -203,7 +204,7 @@ public class TxTest extends AbstractRealmServiceTest { txFailed = true; } - assertTrue("TX should have failed!", txFailed); + assertTrue("TX should have failed as commitOnClose is missing!", txFailed); return ServiceResult.success(); } diff --git a/li.strolch.xmlpers/src/main/java/li/strolch/xmlpers/api/ObjectDao.java b/li.strolch.xmlpers/src/main/java/li/strolch/xmlpers/api/ObjectDao.java index 03d9a3436..e3d66cb6e 100644 --- a/li.strolch.xmlpers/src/main/java/li/strolch/xmlpers/api/ObjectDao.java +++ b/li.strolch.xmlpers/src/main/java/li/strolch/xmlpers/api/ObjectDao.java @@ -33,7 +33,6 @@ import li.strolch.xmlpers.objref.TypeRef; /** * @author Robert von Burg - * */ public class ObjectDao { @@ -54,7 +53,7 @@ public class ObjectDao { assertNotNull(object); PersistenceContext ctx = createCtx(object); ctx.getObjectRef().lock(); - this.objectFilter.add(ctx.getObjectRef().getType(), ctx); + this.objectFilter.add(ctx.getObjectRef().getType(), ctx.getObjectRef(), ctx); } public void addAll(List objects) { @@ -64,7 +63,7 @@ public class ObjectDao { for (T object : objects) { PersistenceContext ctx = createCtx(object); ctx.getObjectRef().lock(); - this.objectFilter.add(ctx.getObjectRef().getType(), ctx); + this.objectFilter.add(ctx.getObjectRef().getType(), ctx.getObjectRef(), ctx); } } } @@ -74,7 +73,7 @@ public class ObjectDao { assertNotNull(object); PersistenceContext ctx = createCtx(object); ctx.getObjectRef().lock(); - this.objectFilter.update(ctx.getObjectRef().getType(), ctx); + this.objectFilter.update(ctx.getObjectRef().getType(), ctx.getObjectRef(), ctx); } public void updateAll(List objects) { @@ -84,7 +83,7 @@ public class ObjectDao { for (T object : objects) { PersistenceContext ctx = createCtx(object); ctx.getObjectRef().lock(); - this.objectFilter.update(ctx.getObjectRef().getType(), ctx); + this.objectFilter.update(ctx.getObjectRef().getType(), ctx.getObjectRef(), ctx); } } } @@ -94,7 +93,7 @@ public class ObjectDao { assertNotNull(object); PersistenceContext ctx = createCtx(object); ctx.getObjectRef().lock(); - this.objectFilter.remove(ctx.getObjectRef().getType(), ctx); + this.objectFilter.remove(ctx.getObjectRef().getType(), ctx.getObjectRef(), ctx); } public void removeAll(List objects) { @@ -104,7 +103,7 @@ public class ObjectDao { for (T object : objects) { PersistenceContext ctx = createCtx(object); ctx.getObjectRef().lock(); - this.objectFilter.remove(ctx.getObjectRef().getType(), ctx); + this.objectFilter.remove(ctx.getObjectRef().getType(), ctx.getObjectRef(), ctx); } } } @@ -132,7 +131,7 @@ public class ObjectDao { PersistenceContext ctx = createCtx(idRef); ctx.getObjectRef().lock(); - this.objectFilter.remove(ctx.getObjectRef().getType(), ctx); + this.objectFilter.remove(ctx.getObjectRef().getType(), ctx.getObjectRef(), ctx); removed++; } } @@ -161,7 +160,7 @@ public class ObjectDao { PersistenceContext ctx = createCtx(idRef); ctx.getObjectRef().lock(); - this.objectFilter.remove(ctx.getObjectRef().getType(), ctx); + this.objectFilter.remove(ctx.getObjectRef().getType(), ctx.getObjectRef(), ctx); removed++; } } finally { @@ -176,7 +175,7 @@ public class ObjectDao { assertIsIdRef(objectRef); PersistenceContext ctx = createCtx(objectRef); ctx.getObjectRef().lock(); - this.objectFilter.remove(objectRef.getType(), ctx); + this.objectFilter.remove(objectRef.getType(), ctx.getObjectRef(), ctx); } public void removeAll(ObjectRef parentRef) { @@ -193,7 +192,7 @@ public class ObjectDao { ObjectRef childRef = parentRef.getChildIdRef(this.tx, id); PersistenceContext ctx = createCtx(childRef); ctx.getObjectRef().lock(); - this.objectFilter.remove(childRef.getType(), ctx); + this.objectFilter.remove(childRef.getType(), ctx.getObjectRef(), ctx); } } finally { parentRef.unlock(); @@ -206,7 +205,7 @@ public class ObjectDao { objectRef.lock(); try { - PersistenceContext ctx = objectRef. createPersistenceContext(this.tx); + PersistenceContext ctx = objectRef.createPersistenceContext(this.tx); return this.fileDao.exists(ctx); } finally { objectRef.unlock(); @@ -219,7 +218,7 @@ public class ObjectDao { objectRef.lock(); try { - PersistenceContext ctx = objectRef. createPersistenceContext(this.tx); + PersistenceContext ctx = objectRef.createPersistenceContext(this.tx); this.fileDao.performRead(ctx); return ctx.getObject(); } finally { @@ -288,18 +287,19 @@ public class ObjectDao { } public PersistenceContext createCtx(T object) { - return this.ctxFactoryDelegator. getCtxFactory(object.getClass()).createCtx(this.tx.getObjectRefCache(), - object); + return this.ctxFactoryDelegator.getCtxFactory(object.getClass()) + .createCtx(this.tx.getObjectRefCache(), object); } public PersistenceContext createCtx(ObjectRef objectRef) { String type = objectRef.getType(); - PersistenceContextFactory ctxFactory = this.ctxFactoryDelegator. getCtxFactory(type); + PersistenceContextFactory ctxFactory = this.ctxFactoryDelegator.getCtxFactory(type); return ctxFactory.createCtx(objectRef); } private void assertNotClosed() { if (!this.tx.isOpen()) - throw new IllegalStateException("Transaction has been closed and thus no operation can be performed!"); //$NON-NLS-1$ + throw new IllegalStateException( + "Transaction has been closed and thus no operation can be performed!"); //$NON-NLS-1$ } }