You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by da...@apache.org on 2013/02/28 16:43:57 UTC

git commit: ISIS-351: fixing action page handling...

Updated Branches:
  refs/heads/master d52c407f1 -> 1e84d8507


ISIS-351: fixing action page handling...

... the previous edit page handling had broken the action page handling.

sad face.

Still, design is improved.  Now have the concept that the IsisTransaction lets the
abortCause (set if a flush(...) triggers a violation error) to be cleared.  This allows
the UI (eg Wicket) to take responsibility for rendering the error.  If it is
not cleared, then the error handling in WebRequestCycleForIsis kicks in.


Project: http://git-wip-us.apache.org/repos/asf/isis/repo
Commit: http://git-wip-us.apache.org/repos/asf/isis/commit/1e84d850
Tree: http://git-wip-us.apache.org/repos/asf/isis/tree/1e84d850
Diff: http://git-wip-us.apache.org/repos/asf/isis/diff/1e84d850

Branch: refs/heads/master
Commit: 1e84d85076bc9d9a08bdeaeb994b0d9804da1848
Parents: d52c407
Author: Dan Haywood <da...@apache.org>
Authored: Thu Feb 28 15:43:40 2013 +0000
Committer: Dan Haywood <da...@apache.org>
Committed: Thu Feb 28 15:43:40 2013 +0000

----------------------------------------------------------------------
 .../integration/wicket/WebRequestCycleForIsis.java |   44 ++++++++++++---
 .../wicket/ui/components/actions/ActionPanel.java  |    6 +-
 .../viewer/wicket/ui/pages/error/ErrorPage.java    |    6 ++-
 .../system/transaction/IsisTransaction.java        |   24 +++++++-
 .../system/transaction/IsisTransactionManager.java |   17 ++++--
 5 files changed, 77 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/isis/blob/1e84d850/component/viewer/wicket/impl/src/main/java/org/apache/isis/viewer/wicket/viewer/integration/wicket/WebRequestCycleForIsis.java
----------------------------------------------------------------------
diff --git a/component/viewer/wicket/impl/src/main/java/org/apache/isis/viewer/wicket/viewer/integration/wicket/WebRequestCycleForIsis.java b/component/viewer/wicket/impl/src/main/java/org/apache/isis/viewer/wicket/viewer/integration/wicket/WebRequestCycleForIsis.java
index 680a8bb..f5d7b0e 100644
--- a/component/viewer/wicket/impl/src/main/java/org/apache/isis/viewer/wicket/viewer/integration/wicket/WebRequestCycleForIsis.java
+++ b/component/viewer/wicket/impl/src/main/java/org/apache/isis/viewer/wicket/viewer/integration/wicket/WebRequestCycleForIsis.java
@@ -38,6 +38,7 @@ import org.apache.wicket.RestartResponseException;
 import org.apache.wicket.Session;
 import org.apache.wicket.core.request.handler.PageProvider;
 import org.apache.wicket.core.request.handler.RenderPageRequestHandler;
+import org.apache.wicket.core.request.handler.RenderPageRequestHandler.RedirectPolicy;
 import org.apache.wicket.protocol.http.WebSession;
 import org.apache.wicket.request.IRequestHandler;
 import org.apache.wicket.request.cycle.AbstractRequestCycleListener;
@@ -74,13 +75,15 @@ public class WebRequestCycleForIsis /*extends WebRequestCycle*/ extends Abstract
         getTransactionManager().startTransaction();
     }
 
-    @Override
-    public synchronized void onEndRequest(RequestCycle cycle) {
-    }
-
-
+    
+    /**
+     * Is called prior to {@link #onEndRequest(RequestCycle)}, and offers the opportunity to
+     * throw an exception.
+     */
     @Override
     public void onRequestHandlerExecuted(RequestCycle cycle, IRequestHandler handler) {
+        LOG.info("onRequestHandlerExecuted: handler: " + handler);
+        
         final IsisSession session = getIsisContext().getSessionInstance();
         if (session != null) {
             try {
@@ -88,17 +91,40 @@ public class WebRequestCycleForIsis /*extends WebRequestCycle*/ extends Abstract
                 // an abort will cause the exception to be thrown.
                 getTransactionManager().endTransaction();
             } catch(Exception ex) {
-                throw new RestartResponseException(errorPageFor(ex));
+                throw new RestartResponseException(errorPageProviderFor(ex), RedirectPolicy.ALWAYS_REDIRECT);
+            }
+        }
+    }
+
+    /**
+     * It is not possible to throw exceptions here, hence use of {@link #onRequestHandlerExecuted(RequestCycle, IRequestHandler)}.
+     */
+    @Override
+    public synchronized void onEndRequest(RequestCycle cycle) {
+        final IsisSession session = getIsisContext().getSessionInstance();
+        if (session != null) {
+            try {
+                // belt and braces
+                getTransactionManager().endTransaction();
             } finally {
                 getIsisContext().closeSessionInstance();
             }
         }
     }
-    
+
+
     @Override
     public IRequestHandler onException(RequestCycle cycle, Exception ex) {
-        final ErrorPage page = errorPageFor(ex);
-        return new RenderPageRequestHandler(new PageProvider(page));
+        // previously we had a handler in here.  However, it seems to be sufficient to just
+        // use the exception handling in the onRequestHandlerExecuted(...) callback
+        // which fires before the onEndRequest(...) callback.
+        
+        //return new RenderPageRequestHandler(errorPageProviderFor(ex));
+        return super.onException(cycle, ex);
+    }
+
+    protected PageProvider errorPageProviderFor(Exception ex) {
+        return new PageProvider(errorPageFor(ex));
     }
 
     protected ErrorPage errorPageFor(Exception ex) {

http://git-wip-us.apache.org/repos/asf/isis/blob/1e84d850/component/viewer/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/actions/ActionPanel.java
----------------------------------------------------------------------
diff --git a/component/viewer/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/actions/ActionPanel.java b/component/viewer/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/actions/ActionPanel.java
index 24057d4..d519f00 100644
--- a/component/viewer/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/actions/ActionPanel.java
+++ b/component/viewer/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/actions/ActionPanel.java
@@ -215,6 +215,8 @@ public class ActionPanel extends PanelAbstract<ActionModel> implements ActionExe
                 // there's no need to abort the transaction, it will have already been done
                 // (in IsisTransactionManager#executeWithinTransaction(...)).
                 
+                getTransactionManager().getTransaction().clearAbortCause();
+                
                 return false;
             } 
             
@@ -308,7 +310,7 @@ public class ActionPanel extends PanelAbstract<ActionModel> implements ActionExe
 
                     // force any changes in state etc to happen now prior to the redirect;
                     // this should cause our page mementos (eg EntityModel) to hold the correct state.  I hope.
-                    IsisContext.getTransactionManager().getTransaction().flush();
+                    panel.getTransactionManager().flushTransaction();
                     
                     // build page, also propogate any concurrency exception that might have occurred already
                     final EntityPage entityPage = new EntityPage(actualAdapter, exIfAny);
@@ -347,8 +349,6 @@ public class ActionPanel extends PanelAbstract<ActionModel> implements ActionExe
             }
 
             private void addOrReplaceCollectionResultsPanel(final ActionPanel panel, final ObjectAdapter resultAdapter) {
-                
-                
                 final EntityCollectionModel collectionModel = EntityCollectionModel.createStandalone(resultAdapter);
                 final SelectionHandler selectionHandler = panel.getModel().getSelectionHandler();
                 if (selectionHandler != null) {

http://git-wip-us.apache.org/repos/asf/isis/blob/1e84d850/component/viewer/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/pages/error/ErrorPage.java
----------------------------------------------------------------------
diff --git a/component/viewer/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/pages/error/ErrorPage.java b/component/viewer/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/pages/error/ErrorPage.java
index 1a8a0e4..679aaf4 100644
--- a/component/viewer/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/pages/error/ErrorPage.java
+++ b/component/viewer/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/pages/error/ErrorPage.java
@@ -19,6 +19,7 @@
 
 package org.apache.isis.viewer.wicket.ui.pages.error;
 
+import java.io.Serializable;
 import java.util.List;
 
 import org.apache.isis.viewer.wicket.ui.pages.PageAbstract;
@@ -57,7 +58,10 @@ public class ErrorPage extends PageAbstract {
 
     private static final JavaScriptResourceReference DIV_TOGGLE_JS = new JavaScriptResourceReference(ErrorPage.class, "div-toggle.js");
 
-    private static class Detail {
+    private static class Detail implements Serializable {
+        
+        private static final long serialVersionUID = 1L;
+        
         enum Type {
             EXCEPTION_CLASS_NAME,
             EXCEPTION_MESSAGE,

http://git-wip-us.apache.org/repos/asf/isis/blob/1e84d850/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransaction.java
----------------------------------------------------------------------
diff --git a/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransaction.java b/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransaction.java
index 81c6671..d20f791 100644
--- a/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransaction.java
+++ b/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransaction.java
@@ -510,19 +510,38 @@ public class IsisTransaction implements TransactionScopedComponent {
     // handle exceptions on load, flush or commit
     /////////////////////////////////////////////////////////////////////////
 
+    @Deprecated
     public void ensureNoAbortCause() {
         Ensure.ensureThatArg(abortCause, is(nullValue()), "abort cause has been set");
     }
 
-    public void setAbortCause(IsisException exception) {
+    
+    
+    /**
+     * Indicate that the transaction must be aborted, and that there is
+     * an unhandled exception to be rendered somehow.
+     * 
+     * <p>
+     * If the cause is subsequently rendered by code higher up the stack, then the
+     * cause can be {@link #clearAbortCause() cleared}.  However, it is not possible
+     * to change the state from {@link State#MUST_ABORT}.
+     */
+    public void setAbortCause(IsisException abortCause) {
         setState(State.MUST_ABORT);
-        abortCause = exception;
+        this.abortCause = abortCause;
     }
     
     public IsisException getAbortCause() {
         return abortCause;
     }
 
+    /**
+     * If the cause has been rendered higher up in the stack, then clear the cause so that
+     * it won't be picked up and rendered elsewhere.
+     */
+    public void clearAbortCause() {
+        abortCause = null;
+    }
 
     
     // //////////////////////////////////////////////////////////
@@ -805,5 +824,6 @@ public class IsisTransaction implements TransactionScopedComponent {
     protected AdapterManager getAdapterManager() {
         return IsisContext.getPersistenceSession().getAdapterManager();
     }
+
     
 }

http://git-wip-us.apache.org/repos/asf/isis/blob/1e84d850/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransactionManager.java
----------------------------------------------------------------------
diff --git a/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransactionManager.java b/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransactionManager.java
index 8fb6aec..7da5f89 100644
--- a/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransactionManager.java
+++ b/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransactionManager.java
@@ -319,14 +319,14 @@ public class IsisTransactionManager implements SessionScopedComponent {
         }
 
         final IsisTransaction transaction = getTransaction();
-        if (transaction == null) {
+        if (transaction == null || transaction.getState().isComplete()) {
+            // allow this method to be called >1 with no adverse affects
             return;
         }
 
         // terminate the transaction early if an abort cause was already set.
         RuntimeException abortCause = this.getTransaction().getAbortCause();
-        if(transaction.getState().mustAbort() || abortCause != null) {
-            // these two checks are, in fact, equivalent.
+        if(transaction.getState().mustAbort()) {
             
             if (LOG.isDebugEnabled()) {
                 LOG.debug("endTransaction: aborting instead [EARLY TERMINATION], abort cause '" + abortCause.getMessage() + "' has been set");
@@ -335,9 +335,16 @@ public class IsisTransactionManager implements SessionScopedComponent {
             
             // just in case any different exception was raised...
             abortCause = this.getTransaction().getAbortCause();
-            throw abortCause;
+            
+            if(abortCause != null) {
+                // hasn't been rendered lower down the stack, so fall back
+                throw abortCause;
+            } else {
+                // assume that any rendering of the problem has been done lower down the stack.
+                return;
+            }
         }
-        
+
         
         transactionLevel--;
         if (transactionLevel == 0) {