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 2014/10/09 21:08:17 UTC
git commit: ISIS-921: disabling safe semantics checking,
and converting Command2 to use a stack.
Repository: isis
Updated Branches:
refs/heads/master 1b10065e7 -> ae3e39a67
ISIS-921: disabling safe semantics checking, and converting Command2 to use a stack.
Project: http://git-wip-us.apache.org/repos/asf/isis/repo
Commit: http://git-wip-us.apache.org/repos/asf/isis/commit/ae3e39a6
Tree: http://git-wip-us.apache.org/repos/asf/isis/tree/ae3e39a6
Diff: http://git-wip-us.apache.org/repos/asf/isis/diff/ae3e39a6
Branch: refs/heads/master
Commit: ae3e39a67c70956c19e2c36018ed8119dcd4c111
Parents: 1b10065
Author: Dan Haywood <da...@haywood-associates.co.uk>
Authored: Thu Oct 9 20:08:07 2014 +0100
Committer: Dan Haywood <da...@haywood-associates.co.uk>
Committed: Thu Oct 9 20:08:07 2014 +0100
----------------------------------------------------------------------
.../isis/applib/services/command/Command2.java | 33 +++++-
.../applib/services/command/CommandDefault.java | 32 ++++--
.../command/Command2ContractTestAbstract.java | 113 +++++++++++++++++++
.../services/command/CommandDefaultTest.java | 9 ++
.../metamodel/facets/InteractionHelper.java | 14 +--
.../persistence/PersistenceConstants.java | 4 +
.../system/transaction/IsisTransaction.java | 91 ++++++++-------
.../main/webapp/WEB-INF/persistor.properties | 5 -
.../main/webapp/WEB-INF/persistor.properties | 5 -
9 files changed, 230 insertions(+), 76 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/isis/blob/ae3e39a6/core/applib/src/main/java/org/apache/isis/applib/services/command/Command2.java
----------------------------------------------------------------------
diff --git a/core/applib/src/main/java/org/apache/isis/applib/services/command/Command2.java b/core/applib/src/main/java/org/apache/isis/applib/services/command/Command2.java
index 0664ba1..f558be4 100644
--- a/core/applib/src/main/java/org/apache/isis/applib/services/command/Command2.java
+++ b/core/applib/src/main/java/org/apache/isis/applib/services/command/Command2.java
@@ -16,6 +16,8 @@
*/
package org.apache.isis.applib.services.command;
+import java.util.List;
+import org.apache.isis.applib.annotation.Programmatic;
import org.apache.isis.applib.services.eventbus.ActionInteractionEvent;
/**
@@ -25,7 +27,7 @@ import org.apache.isis.applib.services.eventbus.ActionInteractionEvent;
public interface Command2 extends Command {
/**
- * The corresponding {@link org.apache.isis.applib.services.eventbus.ActionInteractionEvent}.
+ * The current (most recently pushed) {@link org.apache.isis.applib.services.eventbus.ActionInteractionEvent}.
*
* <p>
* Note that the {@link org.apache.isis.applib.services.eventbus.ActionInteractionEvent} itself is mutable,
@@ -36,14 +38,35 @@ public interface Command2 extends Command {
* </p>
* @return
*/
- ActionInteractionEvent<?> getActionInteractionEvent();
+ @Programmatic
+ ActionInteractionEvent<?> peekActionInteractionEvent();
/**
* <b>NOT API</b>: intended to be called only by the framework.
+ *
+ * <p>
+ * Push a new {@link org.apache.isis.applib.services.eventbus.ActionInteractionEvent}
+ * onto the stack of events held by the command.
+ * </p>
*/
- void setActionInteractionEvent(ActionInteractionEvent<?> event);
-
-
+ @Programmatic
+ void pushActionInteractionEvent(ActionInteractionEvent<?> event);
+ /**
+ * <b>NOT API</b>: intended to be called only by the framework.
+ *
+ * <p>
+ * Push a new {@link org.apache.isis.applib.services.eventbus.ActionInteractionEvent}
+ * onto the stack of events held by the command.
+ * </p>
+ */
+ @Programmatic
+ ActionInteractionEvent<?> popActionInteractionEvent();
+ /**
+ * Returns the {@link org.apache.isis.applib.services.eventbus.ActionInteractionEvent}s in the order that they
+ * were {@link #pushActionInteractionEvent(org.apache.isis.applib.services.eventbus.ActionInteractionEvent) pushed}.
+ */
+ @Programmatic
+ List<ActionInteractionEvent<?>> flushActionInteractionEvents();
}
http://git-wip-us.apache.org/repos/asf/isis/blob/ae3e39a6/core/applib/src/main/java/org/apache/isis/applib/services/command/CommandDefault.java
----------------------------------------------------------------------
diff --git a/core/applib/src/main/java/org/apache/isis/applib/services/command/CommandDefault.java b/core/applib/src/main/java/org/apache/isis/applib/services/command/CommandDefault.java
index 4ee6a4c..95e2f50 100644
--- a/core/applib/src/main/java/org/apache/isis/applib/services/command/CommandDefault.java
+++ b/core/applib/src/main/java/org/apache/isis/applib/services/command/CommandDefault.java
@@ -17,12 +17,13 @@
package org.apache.isis.applib.services.command;
import java.sql.Timestamp;
-import java.util.Map;
-import java.util.UUID;
+import java.util.*;
import java.util.concurrent.atomic.AtomicInteger;
+import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import org.apache.isis.applib.annotation.Command.ExecuteIn;
import org.apache.isis.applib.annotation.Command.Persistence;
+import org.apache.isis.applib.annotation.Programmatic;
import org.apache.isis.applib.services.bookmark.Bookmark;
import org.apache.isis.applib.services.eventbus.ActionInteractionEvent;
import org.apache.isis.applib.util.ObjectContracts;
@@ -185,15 +186,32 @@ public class CommandDefault implements Command2 {
// actionInteractionEvent (property)
// //////////////////////////////////////
- private ActionInteractionEvent actionInteractionEvent;
+ private final LinkedList<ActionInteractionEvent<?>> actionInteractionEvents = Lists.newLinkedList();
+
@Override
- public ActionInteractionEvent<?> getActionInteractionEvent() {
- return actionInteractionEvent;
+ public ActionInteractionEvent<?> peekActionInteractionEvent() {
+ return actionInteractionEvents.isEmpty()? null: actionInteractionEvents.getLast();
}
@Override
- public void setActionInteractionEvent(ActionInteractionEvent<?> event) {
- this.actionInteractionEvent = event;
+ public void pushActionInteractionEvent(ActionInteractionEvent<?> event) {
+ if(peekActionInteractionEvent() == event) {
+ return;
+ }
+ this.actionInteractionEvents.add(event);
+ }
+
+ @Override
+ public ActionInteractionEvent popActionInteractionEvent() {
+ return !actionInteractionEvents.isEmpty() ? actionInteractionEvents.removeLast() : null;
+ }
+
+ @Programmatic
+ public List<ActionInteractionEvent<?>> flushActionInteractionEvents() {
+ final List<ActionInteractionEvent<?>> events =
+ Collections.unmodifiableList(Lists.newArrayList(actionInteractionEvents));
+ actionInteractionEvents.clear();
+ return events;
}
http://git-wip-us.apache.org/repos/asf/isis/blob/ae3e39a6/core/applib/src/test/java/org/apache/isis/applib/services/command/Command2ContractTestAbstract.java
----------------------------------------------------------------------
diff --git a/core/applib/src/test/java/org/apache/isis/applib/services/command/Command2ContractTestAbstract.java b/core/applib/src/test/java/org/apache/isis/applib/services/command/Command2ContractTestAbstract.java
new file mode 100644
index 0000000..8d4aae6
--- /dev/null
+++ b/core/applib/src/test/java/org/apache/isis/applib/services/command/Command2ContractTestAbstract.java
@@ -0,0 +1,113 @@
+package org.apache.isis.applib.services.command;
+
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+import org.apache.isis.applib.services.eventbus.ActionInteractionEvent;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+public abstract class Command2ContractTestAbstract {
+
+ Command2 command;
+
+ ActionInteractionEvent<?> ev1;
+ ActionInteractionEvent<?> ev2;
+
+ @Before
+ public void setUp() throws Exception {
+
+ final Object source = new Object();
+ ev1 = new ActionInteractionEvent.Default(source, null);
+ ev2 = new ActionInteractionEvent.Default(source, null);
+ command = newCommand();
+ }
+
+ protected abstract Command2 newCommand();
+
+ @Test
+ public void givenEmpty() throws Exception {
+ // then
+ assertNull(command.peekActionInteractionEvent());
+ assertNull(command.popActionInteractionEvent());
+ }
+
+ @Test
+ public void givenOne() throws Exception {
+ // given
+ command.pushActionInteractionEvent(ev1);
+
+ // then
+ assertEquals(ev1, command.peekActionInteractionEvent());
+
+ // and when
+ assertEquals(ev1, command.popActionInteractionEvent());
+
+ // then
+ assertNull(command.peekActionInteractionEvent());
+ }
+
+ @Test
+ public void givenTwo() throws Exception {
+ // given
+ command.pushActionInteractionEvent(ev1);
+ command.pushActionInteractionEvent(ev2);
+
+ // then
+ assertEquals(ev2, command.peekActionInteractionEvent());
+
+ // and when
+ assertEquals(ev2, command.popActionInteractionEvent());
+
+ // then
+ assertEquals(ev1, command.peekActionInteractionEvent());
+
+ // and when
+ assertEquals(ev1, command.popActionInteractionEvent());
+
+ // then
+ assertNull(command.peekActionInteractionEvent());
+ }
+
+ @Test
+ public void pushSame() throws Exception {
+
+ // given
+ command.pushActionInteractionEvent(ev1);
+ command.pushActionInteractionEvent(ev1);
+
+ // then
+ assertEquals(ev1, command.peekActionInteractionEvent());
+
+ // and when
+ assertEquals(ev1, command.popActionInteractionEvent());
+
+ // then
+ assertNull(command.peekActionInteractionEvent());
+ }
+
+
+ @Test
+ public void clear() throws Exception {
+
+ // given
+ command.pushActionInteractionEvent(ev1);
+ command.pushActionInteractionEvent(ev2);
+
+ // then
+ assertEquals(ev2, command.peekActionInteractionEvent());
+
+ // and when
+ final List<ActionInteractionEvent<?>> events = command.flushActionInteractionEvents();
+
+ // then
+ assertEquals(ev1, events.get(0));
+ assertEquals(ev2, events.get(1));
+ assertNull(command.peekActionInteractionEvent());
+
+ }
+
+
+
+}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/isis/blob/ae3e39a6/core/applib/src/test/java/org/apache/isis/applib/services/command/CommandDefaultTest.java
----------------------------------------------------------------------
diff --git a/core/applib/src/test/java/org/apache/isis/applib/services/command/CommandDefaultTest.java b/core/applib/src/test/java/org/apache/isis/applib/services/command/CommandDefaultTest.java
new file mode 100644
index 0000000..7a9e185
--- /dev/null
+++ b/core/applib/src/test/java/org/apache/isis/applib/services/command/CommandDefaultTest.java
@@ -0,0 +1,9 @@
+package org.apache.isis.applib.services.command;
+
+public class CommandDefaultTest extends Command2ContractTestAbstract {
+
+ protected Command2 newCommand() {
+ return new CommandDefault();
+ }
+
+}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/isis/blob/ae3e39a6/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/InteractionHelper.java
----------------------------------------------------------------------
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/InteractionHelper.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/InteractionHelper.java
index d0d97ef..a0eb340 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/InteractionHelper.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/InteractionHelper.java
@@ -69,19 +69,7 @@ public class InteractionHelper {
if(command != null && command instanceof Command2) {
final Command2 command2 = (Command2) command;
- // don't overwrite any existing event.
- //
- // this can happen when one action invokes another (wrapped), and can also occur if a
- // bulk action is performed (when the query that produced the list is automatically re-executed).
- // (This logic is in the Wicket viewer's BulkActionsLinkFactory).
- //
- // This also means that the command could refer to a different event (the one of the original
- // outer-most action that started the xactn) compared to the event that references it.
-
- final ActionInteractionEvent<?> actionInteractionEvent = command2.getActionInteractionEvent();
- if(actionInteractionEvent == null) {
- command2.setActionInteractionEvent(event);
- }
+ command2.pushActionInteractionEvent(event);
}
}
} else {
http://git-wip-us.apache.org/repos/asf/isis/blob/ae3e39a6/core/runtime/src/main/java/org/apache/isis/core/runtime/persistence/PersistenceConstants.java
----------------------------------------------------------------------
diff --git a/core/runtime/src/main/java/org/apache/isis/core/runtime/persistence/PersistenceConstants.java b/core/runtime/src/main/java/org/apache/isis/core/runtime/persistence/PersistenceConstants.java
index e8e194c..99a4f45 100644
--- a/core/runtime/src/main/java/org/apache/isis/core/runtime/persistence/PersistenceConstants.java
+++ b/core/runtime/src/main/java/org/apache/isis/core/runtime/persistence/PersistenceConstants.java
@@ -25,10 +25,14 @@ import org.apache.isis.core.runtime.system.persistence.ObjectFactory;
public final class PersistenceConstants {
+ // disabled in ISIS-921, to reinstate in ISIS-922
public static final String ENFORCE_SAFE_SEMANTICS = "isis.persistor.enforceSafeSemantics";
+
/**
* Default is <code>false</code> only for backward compatibility (to avoid lots of breakages in existing code);
* in future might change to <code>true</code>.
+ *
+ * disabled in ISIS-921, to reinstate in ISIS-922
*/
public static final boolean ENFORCE_SAFE_SEMANTICS_DEFAULT = false;
http://git-wip-us.apache.org/repos/asf/isis/blob/ae3e39a6/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 9f52c24..dfb1925 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
@@ -34,7 +34,6 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.isis.applib.Identifier;
import org.apache.isis.applib.RecoverableException;
-import org.apache.isis.applib.annotation.ActionSemantics;
import org.apache.isis.applib.annotation.Bulk;
import org.apache.isis.applib.annotation.PublishedAction;
import org.apache.isis.applib.annotation.PublishedObject;
@@ -113,7 +112,7 @@ public class IsisTransaction implements TransactionScopedComponent {
* <p>
* May {@link IsisTransaction#flush() flush},
* {@link IsisTransaction#commit() commit} or
- * {@link IsisTransaction#abort() abort}.
+ * {@link IsisTransaction#markAsAborted() abort}.
*/
IN_PROGRESS(TransactionState.IN_PROGRESS),
/**
@@ -123,7 +122,7 @@ public class IsisTransaction implements TransactionScopedComponent {
* May not {@link IsisTransaction#flush()} or
* {@link IsisTransaction#commit() commit} (will throw an
* {@link IllegalStateException}), but can only
- * {@link IsisTransaction#abort() abort}.
+ * {@link IsisTransaction#markAsAborted() abort}.
*
* <p>
* Similar to <tt>setRollbackOnly</tt> in EJBs.
@@ -134,7 +133,7 @@ public class IsisTransaction implements TransactionScopedComponent {
*
* <p>
* May not {@link IsisTransaction#flush()} or
- * {@link IsisTransaction#abort() abort} or
+ * {@link IsisTransaction#markAsAborted() abort}.
* {@link IsisTransaction#commit() commit} (will throw
* {@link IllegalStateException}).
*/
@@ -145,7 +144,7 @@ public class IsisTransaction implements TransactionScopedComponent {
* <p>
* May not {@link IsisTransaction#flush()},
* {@link IsisTransaction#commit() commit} or
- * {@link IsisTransaction#abort() abort} (will throw
+ * {@link IsisTransaction#markAsAborted() abort} (will throw
* {@link IllegalStateException}).
*/
ABORTED(TransactionState.ABORTED);
@@ -754,53 +753,63 @@ public class IsisTransaction implements TransactionScopedComponent {
}
private void ensureSafeSemanticsHonoured(Command command, Set<ObjectAdapter> changedAdapters) {
+
+ if(true) {
+
+ // ISIS-921: disabling this functionality...
+ //
+ // ... the issue is that an edit (which mutates state, obviously), can cause a contributed property to
+ // be evaluated, which has safe semantics.
+ //
+ // the solution, I think, is to set up some sort of "dummy" action to represent the edit.
+ // this needs to be installed pretty early up in the stack trace. ISIS-922 raised for this.
+ //
+
+ return;
+ }
+
if (!(command instanceof Command2)) {
return;
}
final Command2 command2 = (Command2) command;
- final ActionInteractionEvent<?> event = command2.getActionInteractionEvent();
- if (event == null) {
+ final List<ActionInteractionEvent<?>> events = command2.flushActionInteractionEvents();
+ if (events.isEmpty()) {
return;
}
- try {
- final ActionSemantics.Of actionSemantics = event.getActionSemantics();
- if(actionSemantics == null) {
- return;
- }
- if (!actionSemantics.isSafe()) {
+
+ // are all safe?
+ for (ActionInteractionEvent<?> event : events) {
+ if(!event.getActionSemantics().isSafe()) {
+ // found at least one non-safe action, so all bets are off.
return;
}
+ }
- if(!changedAdapters.isEmpty()) {
- final String msg = "Action '" + event.getIdentifier().toFullIdentityString() + "'" +
- " (with safe semantics)" +
- " caused " + changedAdapters.size() + " object" + (changedAdapters.size() != 1 ? "s" : "") +
- " to be modified";
- LOG.error(msg);
- for (ObjectAdapter changedAdapter : changedAdapters) {
- final StringBuilder builder = new StringBuilder(" > ")
- .append(changedAdapter.getSpecification().getFullIdentifier())
- .append(": ");
- if(!changedAdapter.isDestroyed()) {
- builder.append(changedAdapter.titleString(null));
- } else {
- builder.append("(deleted object)");
- }
- LOG.error(builder.toString());
- }
+ // all actions invoked had safe semantics; were any objects changed?
+ if (changedAdapters.isEmpty()) {
+ return;
+ }
- final boolean enforceSafeSemantics = getConfiguration().getBoolean(PersistenceConstants.ENFORCE_SAFE_SEMANTICS, PersistenceConstants.ENFORCE_SAFE_SEMANTICS_DEFAULT);
- if(enforceSafeSemantics) {
- throw new RecoverableException(msg);
- }
+ final String msg = "Action '" + events.get(0).getIdentifier().toFullIdentityString() + "'" +
+ " (with safe semantics)" +
+ " caused " + changedAdapters.size() + " object" + (changedAdapters.size() != 1 ? "s" : "") +
+ " to be modified";
+ LOG.error(msg);
+ for (ObjectAdapter changedAdapter : changedAdapters) {
+ final StringBuilder builder = new StringBuilder(" > ")
+ .append(changedAdapter.getSpecification().getFullIdentifier())
+ .append(": ");
+ if(!changedAdapter.isDestroyed()) {
+ builder.append(changedAdapter.titleString(null));
+ } else {
+ builder.append("(deleted object)");
}
- } finally {
- // irrespective of outcome, clear out the event.
- // this has no effect in the production app, but benefits integration tests
- // where otherwise the event from a preceding xactn (eg the action in the 'when') can "leak" into
- // pertaining to the next phase (eg the assertions in the 'then').
+ LOG.error(builder.toString());
+ }
- command2.setActionInteractionEvent(null);
+ final boolean enforceSafeSemantics = getConfiguration().getBoolean(PersistenceConstants.ENFORCE_SAFE_SEMANTICS, PersistenceConstants.ENFORCE_SAFE_SEMANTICS_DEFAULT);
+ if(enforceSafeSemantics) {
+ throw new RecoverableException(msg);
}
}
@@ -869,7 +878,7 @@ public class IsisTransaction implements TransactionScopedComponent {
if(command instanceof Command2) {
final Command2 command2 = (Command2) command;
- command2.setActionInteractionEvent(null);
+ command2.flushActionInteractionEvents();
}
}
}
http://git-wip-us.apache.org/repos/asf/isis/blob/ae3e39a6/example/application/simpleapp/webapp/src/main/webapp/WEB-INF/persistor.properties
----------------------------------------------------------------------
diff --git a/example/application/simpleapp/webapp/src/main/webapp/WEB-INF/persistor.properties b/example/application/simpleapp/webapp/src/main/webapp/WEB-INF/persistor.properties
index 6c9e64e..a4bffa6 100644
--- a/example/application/simpleapp/webapp/src/main/webapp/WEB-INF/persistor.properties
+++ b/example/application/simpleapp/webapp/src/main/webapp/WEB-INF/persistor.properties
@@ -24,11 +24,6 @@
#################################################################################
-# whether to prevent a transaction initiated from an action with safe semantics
-# from ever changing any objects (intentionally or otherwise)
-isis.persistor.enforceSafeSemantics=true
-
-
# generally speaking this should not be enabled
isis.persistor.disableConcurrencyChecking=false
http://git-wip-us.apache.org/repos/asf/isis/blob/ae3e39a6/example/application/todoapp/webapp/src/main/webapp/WEB-INF/persistor.properties
----------------------------------------------------------------------
diff --git a/example/application/todoapp/webapp/src/main/webapp/WEB-INF/persistor.properties b/example/application/todoapp/webapp/src/main/webapp/WEB-INF/persistor.properties
index 982997e..18ea3a1 100644
--- a/example/application/todoapp/webapp/src/main/webapp/WEB-INF/persistor.properties
+++ b/example/application/todoapp/webapp/src/main/webapp/WEB-INF/persistor.properties
@@ -23,11 +23,6 @@
#################################################################################
-# whether to prevent a transaction initiated from an action with safe semantics
-# from ever changing any objects (intentionally or otherwise)
-isis.persistor.enforceSafeSemantics=true
-
-
# generally speaking this should not be enabled
isis.persistor.disableConcurrencyChecking=false