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 2018/02/03 12:34:49 UTC

[isis] 01/03: ISIS-1569: CommandExecutionAbstract delegates to new CommandExecutorService. Also

This is an automated email from the ASF dual-hosted git repository.

danhaywood pushed a commit to branch maint-1.16.1
in repository https://gitbox.apache.org/repos/asf/isis.git

commit 8540001f3304f319a052bbe70aa77b74cbc349f6
Author: Dan Haywood <da...@haywood-associates.co.uk>
AuthorDate: Sat Feb 3 11:47:45 2018 +0000

    ISIS-1569: CommandExecutionAbstract delegates to new CommandExecutorService.  Also
    
    also:
    - adds additional overrides for setting the Clock using time-millis
    - remove command reification for some of the Object_ mixins
---
 .../java/org/apache/isis/applib/clock/Clock.java   |   6 +-
 .../isis/applib/fixtures/TickingFixtureClock.java  |   6 +-
 .../services/layout/Object_rebuildMetamodel.java   |   4 +-
 .../background/BackgroundCommandExecution.java     |   4 +-
 .../background/CommandExecutionAbstract.java       | 411 +--------------------
 .../background/CommandExecutorService.java         |  44 +++
 ...act.java => CommandExecutorServiceDefault.java} | 314 +++++++---------
 .../wicket/viewer/services/Object_clearHints.java  |   4 +-
 8 files changed, 212 insertions(+), 581 deletions(-)

diff --git a/core/applib/src/main/java/org/apache/isis/applib/clock/Clock.java b/core/applib/src/main/java/org/apache/isis/applib/clock/Clock.java
index cd7c6c9..8f4ff9f 100644
--- a/core/applib/src/main/java/org/apache/isis/applib/clock/Clock.java
+++ b/core/applib/src/main/java/org/apache/isis/applib/clock/Clock.java
@@ -125,7 +125,11 @@ public abstract class Clock {
 
 
     public static Timestamp getTimeAsJavaSqlTimestamp() {
-        return new java.sql.Timestamp(getTimeAsDateTime().getMillis());
+        return new java.sql.Timestamp(getTimeAsMillis());
+    }
+
+    public static long getTimeAsMillis() {
+        return getTimeAsDateTime().getMillis();
     }
 
     /**
diff --git a/core/applib/src/main/java/org/apache/isis/applib/fixtures/TickingFixtureClock.java b/core/applib/src/main/java/org/apache/isis/applib/fixtures/TickingFixtureClock.java
index 5959b78..6079b58 100644
--- a/core/applib/src/main/java/org/apache/isis/applib/fixtures/TickingFixtureClock.java
+++ b/core/applib/src/main/java/org/apache/isis/applib/fixtures/TickingFixtureClock.java
@@ -121,7 +121,11 @@ public class TickingFixtureClock extends Clock {
     }
 
     public void setTime(final Timestamp timestamp) {
-        calendar.setTimeInMillis(timestamp.getTime());
+        setTime(timestamp.getTime());
+    }
+
+    public void setTime(final long millis) {
+        calendar.setTimeInMillis(millis);
     }
 
     /**
diff --git a/core/applib/src/main/java/org/apache/isis/applib/services/layout/Object_rebuildMetamodel.java b/core/applib/src/main/java/org/apache/isis/applib/services/layout/Object_rebuildMetamodel.java
index 92212dd..3ee9e5d 100644
--- a/core/applib/src/main/java/org/apache/isis/applib/services/layout/Object_rebuildMetamodel.java
+++ b/core/applib/src/main/java/org/apache/isis/applib/services/layout/Object_rebuildMetamodel.java
@@ -18,6 +18,7 @@ package org.apache.isis.applib.services.layout;
 
 import org.apache.isis.applib.annotation.Action;
 import org.apache.isis.applib.annotation.ActionLayout;
+import org.apache.isis.applib.annotation.CommandReification;
 import org.apache.isis.applib.annotation.Contributed;
 import org.apache.isis.applib.annotation.MemberOrder;
 import org.apache.isis.applib.annotation.Mixin;
@@ -39,7 +40,8 @@ public class Object_rebuildMetamodel {
     @Action(
             domainEvent = ActionDomainEvent.class,
             semantics = SemanticsOf.IDEMPOTENT,
-            restrictTo = RestrictTo.PROTOTYPING
+            restrictTo = RestrictTo.PROTOTYPING,
+            command = CommandReification.DISABLED
     )
     @ActionLayout(
             contributed = Contributed.AS_ACTION,
diff --git a/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/BackgroundCommandExecution.java b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/BackgroundCommandExecution.java
index f42bd6c..d83c620 100644
--- a/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/BackgroundCommandExecution.java
+++ b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/BackgroundCommandExecution.java
@@ -43,10 +43,10 @@ public abstract class BackgroundCommandExecution extends CommandExecutionAbstrac
      * Defaults to the historical defaults * for running background commands.
      */
     public BackgroundCommandExecution() {
-        this(SudoPolicy.NO_SWITCH);
+        this(CommandExecutorService.SudoPolicy.NO_SWITCH);
     }
 
-    public BackgroundCommandExecution(final SudoPolicy sudoPolicy) {
+    public BackgroundCommandExecution(final CommandExecutorService.SudoPolicy sudoPolicy) {
         super(sudoPolicy);
     }
 
diff --git a/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/CommandExecutionAbstract.java b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/CommandExecutionAbstract.java
index 188a9e2..56fc015 100644
--- a/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/CommandExecutionAbstract.java
+++ b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/CommandExecutionAbstract.java
@@ -16,53 +16,12 @@
  */
 package org.apache.isis.core.runtime.services.background;
 
-import java.sql.Timestamp;
-import java.util.Collections;
-import java.util.List;
-import java.util.concurrent.Callable;
-
-import com.google.common.base.Function;
-import com.google.common.base.Throwables;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import org.apache.isis.applib.services.background.ActionInvocationMemento;
-import org.apache.isis.applib.services.bookmark.Bookmark;
-import org.apache.isis.applib.services.bookmark.BookmarkService2;
-import org.apache.isis.applib.services.clock.ClockService;
 import org.apache.isis.applib.services.command.Command;
-import org.apache.isis.applib.services.command.Command.Executor;
-import org.apache.isis.applib.services.iactn.Interaction;
-import org.apache.isis.applib.services.iactn.InteractionContext;
-import org.apache.isis.applib.services.jaxb.JaxbService;
-import org.apache.isis.applib.services.sudo.SudoService;
-import org.apache.isis.core.metamodel.adapter.ObjectAdapter;
-import org.apache.isis.core.metamodel.consent.InteractionInitiatedBy;
-import org.apache.isis.core.metamodel.facets.actions.action.invocation.CommandUtil;
-import org.apache.isis.core.metamodel.spec.ObjectSpecification;
-import org.apache.isis.core.metamodel.spec.feature.Contributed;
-import org.apache.isis.core.metamodel.spec.feature.ObjectAction;
-import org.apache.isis.core.metamodel.spec.feature.ObjectAssociation;
-import org.apache.isis.core.metamodel.spec.feature.OneToOneAssociation;
 import org.apache.isis.core.runtime.sessiontemplate.AbstractIsisSessionTemplate;
 import org.apache.isis.core.runtime.system.transaction.IsisTransactionManager;
-import org.apache.isis.core.runtime.system.transaction.TransactionalClosure;
-import org.apache.isis.core.runtime.system.transaction.TransactionalClosureWithReturn;
-import org.apache.isis.schema.cmd.v1.ActionDto;
-import org.apache.isis.schema.cmd.v1.CommandDto;
-import org.apache.isis.schema.cmd.v1.MemberDto;
-import org.apache.isis.schema.cmd.v1.ParamDto;
-import org.apache.isis.schema.cmd.v1.ParamsDto;
-import org.apache.isis.schema.cmd.v1.PropertyDto;
-import org.apache.isis.schema.common.v1.InteractionType;
-import org.apache.isis.schema.common.v1.OidDto;
-import org.apache.isis.schema.common.v1.OidsDto;
-import org.apache.isis.schema.common.v1.ValueWithTypeDto;
-import org.apache.isis.schema.utils.CommandDtoUtils;
-import org.apache.isis.schema.utils.CommonDtoUtils;
 
 /**
  */
@@ -70,388 +29,40 @@ public abstract class CommandExecutionAbstract extends AbstractIsisSessionTempla
 
     private final static Logger LOG = LoggerFactory.getLogger(CommandExecutionAbstract.class);
 
-    public enum SudoPolicy {
-        /**
-         * For example, regular background commands.
-         */
-        NO_SWITCH,
-        /**
-         * For example, replayable commands.
-         */
-        SWITCH,
-    }
-
-    private final SudoPolicy sudoPolicy;
+    private final CommandExecutorService.SudoPolicy sudoPolicy;
 
-    protected CommandExecutionAbstract(final SudoPolicy sudoPolicy) {
+    protected CommandExecutionAbstract(final CommandExecutorService.SudoPolicy sudoPolicy) {
         this.sudoPolicy = sudoPolicy;
     }
 
-    // //////////////////////////////////////
+
 
     /**
-     * Executes the command within a transaction, and with respect to the specified {@link SudoPolicy}
+     * Executes the command within a transaction, and with respect to the {@link CommandExecutorService.SudoPolicy}
      * specified in the constructor.
-     * 
-     * <p>
-     *     Intended to be called from an override of {@link #doExecute(Object)},
-     *     not from {@link #doExecuteWithTransaction(Object)} (because the error handling is different).
-     * </p>
      *
      * <p>
-     *     Subclasses can add additional policies (eg adjusting the clock) by overriding {@link #executeCommand(IsisTransactionManager, Command)} and delegating to {@link #doExecuteCommand(IsisTransactionManager, Command)} as required.
+     *     Uses {@link CommandExecutorService} to actually execute the command.
      * </p>
-     *
-     * @return - any exception arising.  We don't throw this exception, because an exception might be what is expected (eg if replaying a command that itself failed to execute).
      */
-    protected final Exception execute(
-            final IsisTransactionManager transactionManager,
-            final Command command) {
-
-        try {
-            return executeCommandWithinTran(transactionManager, command);
-        } catch (final RuntimeException ex) {
-
-            // attempting to commit the xactn itself could cause an issue
-            // so we need extra exception handling here, it seems.
-
-            org.apache.isis.applib.annotation.Command.ExecuteIn executeIn = command.getExecuteIn();
-            LOG.warn("Exception when committing for: {} {}", executeIn, command.getMemberIdentifier(), ex);
-
-            //
-            // the previous transaction will have been aborted as part of the recovery handling within
-            // TransactionManager's executeCommandWithinTran
-            //
-            // we therefore start a new xactn in order to make sure that this background command is marked as a failure
-            // so it won't be attempted again.
-            //
-            transactionManager.executeWithinTransaction(new TransactionalClosure(){
-                @Override
-                public void execute() {
-
-                    final Interaction backgroundInteraction = interactionContext.getInteraction();
-                    final Interaction.Execution currentExecution = backgroundInteraction.getCurrentExecution();
-                    command.setStartedAt(
-                            currentExecution != null
-                                    ? currentExecution.getStartedAt()
-                                    : clockService.nowAsJavaSqlTimestamp());
-
-                    command.setCompletedAt(clockService.nowAsJavaSqlTimestamp());
-                    command.setException(Throwables.getStackTraceAsString(ex));
-                }
-            });
-
-            return ex;
-        }
-    }
-
-    private Exception executeCommandWithinTran(
+    protected final void execute(
             final IsisTransactionManager transactionManager,
             final Command command) {
 
-        return transactionManager.executeWithinTransaction(
-                command,
-                new TransactionalClosureWithReturn<Exception>() {
-            @Override
-            public Exception execute() {
+        transactionManager.startTransaction();
 
-                return executeCommandPerSudoPolicy(transactionManager, command);
-            }
-        });
-    }
+        // the executor service will handle any exceptions thrown.
+        commandExecutorService.executeCommand(sudoPolicy, command);
 
-    private Exception executeCommandPerSudoPolicy(
-            final IsisTransactionManager transactionManager,
-            final Command command) {
+        transactionManager.endTransaction();
 
-        switch (sudoPolicy) {
-        case NO_SWITCH:
-            return executeCommand(transactionManager, command);
-        case SWITCH:
-            final String user = command.getUser();
-            return sudoService.sudo(user, new Callable<Exception>() {
-                @Override
-                public Exception call() {
-                    return executeCommand(transactionManager, command);
-                }
-            });
-        default:
-            throw new IllegalStateException("Unrecognized sudoPolicy: " + sudoPolicy);
-        }
     }
 
-    /**
-     * Simply delegates to {@link #doExecuteCommand(IsisTransactionManager, Command)}.
-     *
-     * Overridable, so execution policy can be adjusted if required,
-     * eg when replaying, adjust the clock before executing the command.
-     *
-     * @return - any exception arising
-     */
-    protected Exception executeCommand(
-            final IsisTransactionManager transactionManager,
-            final Command command) {
-        return doExecuteCommand(transactionManager, command);
-    }
-
-    /**
-     * Not overrideable, but intended to be called by
-     * {@link #executeCommand(IsisTransactionManager, Command)} (which is).
-     *
-     * @return - any exception arising.  We don't throw this exception, because an exception might be
-     *           what is expected (eg if replaying a command that itself failed to execute).
-     */
-    protected final Exception doExecuteCommand(
-            final IsisTransactionManager transactionManager,
-            final Command command) {
-
-        // setup for us by IsisTransactionManager; will have the transactionId of the backgroundCommand
-        final Interaction backgroundInteraction = interactionContext.getInteraction();
-
-        final String memento = command.getMemento();
-
-        org.apache.isis.applib.annotation.Command.ExecuteIn executeIn = command.getExecuteIn();
-
-        LOG.info("Executing: {} {}", executeIn, command.getMemberIdentifier());
-
-        RuntimeException exceptionIfAny = null;
-
-        try {
-            command.setExecutor(Executor.BACKGROUND);
-
-            // responsibility for setting the Command#startedAt is in the ActionInvocationFacet or
-            // PropertySetterFacet, but this is run if the domain object was found.  If the domain object is
-            // thrown then we would have a command with only completedAt, which is inconsistent.
-            // Therefore instead we copy down from the backgroundInteraction (similar to how we populate the
-            // completedAt at the end)
-            final Interaction.Execution priorExecution = backgroundInteraction.getPriorExecution();
-
-            final Timestamp startedAt = priorExecution != null
-                    ? priorExecution.getStartedAt()
-                    : clockService.nowAsJavaSqlTimestamp();
-            final Timestamp completedAt =
-                    priorExecution != null
-                            ? priorExecution.getCompletedAt()
-                            : clockService.nowAsJavaSqlTimestamp();  // close enough...
-
-            command.setStartedAt(startedAt);
-            command.setCompletedAt(completedAt);
-
-            final CommandDto dto = jaxbService.fromXml(CommandDto.class, memento);
-
-            final MemberDto memberDto = dto.getMember();
-            final String memberId = memberDto.getMemberIdentifier();
-
-            final OidsDto oidsDto = CommandDtoUtils.targetsFor(dto);
-            final List<OidDto> targetOidDtos = oidsDto.getOid();
-
-            final InteractionType interactionType = memberDto.getInteractionType();
-            if(interactionType == InteractionType.ACTION_INVOCATION) {
-
-                final ActionDto actionDto = (ActionDto) memberDto;
-
-                for (OidDto targetOidDto : targetOidDtos) {
 
-                    final ObjectAdapter targetAdapter = adapterFor(targetOidDto);
-                    final ObjectAction objectAction = findObjectAction(targetAdapter, memberId);
 
-                    // we pass 'null' for the mixedInAdapter; if this action _is_ a mixin then
-                    // it will switch the targetAdapter to be the mixedInAdapter transparently
-                    final ObjectAdapter[] argAdapters = argAdaptersFor(actionDto);
-                    final ObjectAdapter resultAdapter = objectAction.execute(
-                            targetAdapter, null, argAdapters, InteractionInitiatedBy.FRAMEWORK);
-
-                    //
-                    // for the result adapter, we could alternatively have used...
-                    // (priorExecution populated by the push/pop within the interaction object)
-                    //
-                    // final Interaction.Execution priorExecution = backgroundInteraction.getPriorExecution();
-                    // Object unused = priorExecution.getReturned();
-                    //
-
-                    // REVIEW: this doesn't really make sense if >1 action
-                    // in any case, the capturing of the action interaction should be the
-                    // responsibility of auditing/profiling
-                    if(resultAdapter != null) {
-                        Bookmark resultBookmark = CommandUtil.bookmarkFor(resultAdapter);
-                        command.setResult(resultBookmark);
-                    }
-                }
-            } else {
-
-                final PropertyDto propertyDto = (PropertyDto) memberDto;
-
-                for (OidDto targetOidDto : targetOidDtos) {
-
-                    final Bookmark bookmark = Bookmark.from(targetOidDto);
-                    final Object targetObject = bookmarkService.lookup(bookmark);
-
-                    final ObjectAdapter targetAdapter = adapterFor(targetObject);
-
-                    final OneToOneAssociation property = findOneToOneAssociation(targetAdapter, memberId);
-
-                    final ObjectAdapter newValueAdapter = newValueAdapterFor(propertyDto);
-
-                    property.set(targetAdapter, newValueAdapter, InteractionInitiatedBy.FRAMEWORK);
-                    // there is no return value for property modifications.
-                }
-            }
-
-        } catch (RuntimeException ex) {
-
-            LOG.warn("Exception for: {} {}", executeIn, command.getMemberIdentifier(), ex);
-
-            // hmmm, this doesn't really make sense if >1 action
-            //
-            // in any case, the capturing of the result of the action invocation should be the
-            // responsibility of the interaction...
-            command.setException(Throwables.getStackTraceAsString(ex));
-
-            // lower down the stack the IsisTransactionManager will have set the transaction to abort
-            // however, we don't want that to occur (because any changes made to the backgroundCommand itself
-            // would also be rolled back, and it would keep getting picked up again by a scheduler for
-            // processing); instead we clear the abort cause and ensure we can continue.
-            transactionManager.getCurrentTransaction().clearAbortCauseAndContinue();
-
-            // checked at the end
-            exceptionIfAny = ex;
-
-        }
-
-        // it's possible that there is no priorExecution, specifically if there was an exception
-        // invoking the action.  We therefore need to guard that case.
-        final Interaction.Execution priorExecution = backgroundInteraction.getPriorExecution();
-        final Timestamp completedAt =
-                priorExecution != null
-                        ? priorExecution.getCompletedAt()
-                        : clockService.nowAsJavaSqlTimestamp();  // close enough...
-        command.setCompletedAt(completedAt);
-
-        return exceptionIfAny;
-    }
-
-    private static ObjectAction findObjectAction(
-            final ObjectAdapter targetAdapter,
-            final String actionId) throws RuntimeException {
-
-        final ObjectSpecification specification = targetAdapter.getSpecification();
-
-        final ObjectAction objectAction = findActionElseNull(specification, actionId);
-        if(objectAction == null) {
-            throw new RuntimeException(String.format("Unknown action '%s'", actionId));
-        }
-        return objectAction;
-    }
-
-    private static OneToOneAssociation findOneToOneAssociation(
-            final ObjectAdapter targetAdapter,
-            final String propertyId) throws RuntimeException {
-
-        final ObjectSpecification specification = targetAdapter.getSpecification();
-
-        final OneToOneAssociation property = findOneToOneAssociationElseNull(specification, propertyId);
-        if(property == null) {
-            throw new RuntimeException(String.format("Unknown property '%s'", propertyId));
-        }
-        return property;
-    }
-
-    private ObjectAdapter newValueAdapterFor(final PropertyDto propertyDto) {
-        final ValueWithTypeDto newValue = propertyDto.getNewValue();
-        final Object arg = CommonDtoUtils.getValue(newValue);
-        return adapterFor(arg);
-    }
-
-    private static ObjectAction findActionElseNull(
-            final ObjectSpecification specification,
-            final String actionId) {
-        final List<ObjectAction> objectActions = specification.getObjectActions(Contributed.INCLUDED);
-        for (final ObjectAction objectAction : objectActions) {
-            if(objectAction.getIdentifier().toClassAndNameIdentityString().equals(actionId)) {
-                return objectAction;
-            }
-        }
-        return null;
-    }
-
-    private static OneToOneAssociation findOneToOneAssociationElseNull(
-            final ObjectSpecification specification,
-            final String propertyId) {
-        final List<ObjectAssociation> associations = specification.getAssociations(Contributed.INCLUDED);
-        for (final ObjectAssociation association : associations) {
-            if( association.getIdentifier().toClassAndNameIdentityString().equals(propertyId) &&
-                association instanceof OneToOneAssociation) {
-                return (OneToOneAssociation) association;
-            }
-        }
-        return null;
-    }
-
-    private ObjectAdapter[] argAdaptersFor(final ActionInvocationMemento aim)  {
-        final int numArgs = aim.getNumArgs();
-        final List<ObjectAdapter> argumentAdapters = Lists.newArrayList();
-        for(int i=0; i<numArgs; i++) {
-            final ObjectAdapter argAdapter = argAdapterFor(aim, i);
-            argumentAdapters.add(argAdapter);
-        }
-        return argumentAdapters.toArray(new ObjectAdapter[]{});
-    }
-
-    private ObjectAdapter argAdapterFor(final ActionInvocationMemento aim, int num) {
-        final Class<?> argType;
-        try {
-            argType = aim.getArgType(num);
-            final Object arg = aim.getArg(num, argType);
-            if(arg == null) {
-                return null;
-            }
-            return adapterFor(arg);
-
-        } catch (ClassNotFoundException e) {
-            throw new RuntimeException(e);
-        }
-    }
-
-    private ObjectAdapter[] argAdaptersFor(final ActionDto actionDto) {
-        final List<ParamDto> params = paramDtosFrom(actionDto);
-        final List<ObjectAdapter> args = Lists.newArrayList(
-                Iterables.transform(params, new Function<ParamDto, ObjectAdapter>() {
-                    @Override
-                    public ObjectAdapter apply(final ParamDto paramDto) {
-                        final Object arg = CommonDtoUtils.getValue(paramDto);
-                        return adapterFor(arg);
-                    }
-                })
-        );
-        return args.toArray(new ObjectAdapter[]{});
-    }
-
-    private static List<ParamDto> paramDtosFrom(final ActionDto actionDto) {
-        final ParamsDto parameters = actionDto.getParameters();
-        if (parameters != null) {
-            final List<ParamDto> parameterList = parameters.getParameter();
-            if (parameterList != null) {
-                return parameterList;
-            }
-        }
-        return Collections.emptyList();
-    }
 
     // //////////////////////////////////////
 
     @javax.inject.Inject
-    BookmarkService2 bookmarkService;
-
-    @javax.inject.Inject
-    JaxbService jaxbService;
-
-    @javax.inject.Inject
-    InteractionContext interactionContext;
-
-    @javax.inject.Inject
-    SudoService sudoService;
-
-    @javax.inject.Inject
-    ClockService clockService;
-
+    CommandExecutorService commandExecutorService;
 }
diff --git a/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/CommandExecutorService.java b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/CommandExecutorService.java
new file mode 100644
index 0000000..bfbf153
--- /dev/null
+++ b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/CommandExecutorService.java
@@ -0,0 +1,44 @@
+/**
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.apache.isis.core.runtime.services.background;
+
+import org.apache.isis.applib.annotation.Programmatic;
+import org.apache.isis.applib.services.command.Command;
+
+public interface CommandExecutorService {
+
+    enum SudoPolicy {
+        /**
+         * For example, regular background commands.
+         */
+        NO_SWITCH,
+        /**
+         * For example, replayable commands.
+         */
+        SWITCH,
+    }
+
+    /**
+     *
+     * @param sudoPolicy
+     * @param command
+     * @return - any exception raised by the command.
+     */
+    @Programmatic
+    void executeCommand(SudoPolicy sudoPolicy, Command command);
+
+}
diff --git a/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/CommandExecutionAbstract.java b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/CommandExecutorServiceDefault.java
similarity index 60%
copy from core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/CommandExecutionAbstract.java
copy to core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/CommandExecutorServiceDefault.java
index 188a9e2..9791603 100644
--- a/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/CommandExecutionAbstract.java
+++ b/core/runtime/src/main/java/org/apache/isis/core/runtime/services/background/CommandExecutorServiceDefault.java
@@ -19,7 +19,6 @@ package org.apache.isis.core.runtime.services.background;
 import java.sql.Timestamp;
 import java.util.Collections;
 import java.util.List;
-import java.util.concurrent.Callable;
 
 import com.google.common.base.Function;
 import com.google.common.base.Throwables;
@@ -29,17 +28,22 @@ import com.google.common.collect.Lists;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import org.apache.isis.applib.services.background.ActionInvocationMemento;
+import org.apache.isis.applib.annotation.DomainService;
+import org.apache.isis.applib.annotation.NatureOfService;
+import org.apache.isis.applib.annotation.Programmatic;
 import org.apache.isis.applib.services.bookmark.Bookmark;
 import org.apache.isis.applib.services.bookmark.BookmarkService2;
 import org.apache.isis.applib.services.clock.ClockService;
 import org.apache.isis.applib.services.command.Command;
-import org.apache.isis.applib.services.command.Command.Executor;
 import org.apache.isis.applib.services.iactn.Interaction;
 import org.apache.isis.applib.services.iactn.InteractionContext;
 import org.apache.isis.applib.services.jaxb.JaxbService;
 import org.apache.isis.applib.services.sudo.SudoService;
+import org.apache.isis.applib.services.xactn.Transaction2;
+import org.apache.isis.applib.services.xactn.TransactionService3;
+import org.apache.isis.applib.services.xactn.TransactionState;
 import org.apache.isis.core.metamodel.adapter.ObjectAdapter;
+import org.apache.isis.core.metamodel.adapter.oid.RootOid;
 import org.apache.isis.core.metamodel.consent.InteractionInitiatedBy;
 import org.apache.isis.core.metamodel.facets.actions.action.invocation.CommandUtil;
 import org.apache.isis.core.metamodel.spec.ObjectSpecification;
@@ -47,166 +51,76 @@ import org.apache.isis.core.metamodel.spec.feature.Contributed;
 import org.apache.isis.core.metamodel.spec.feature.ObjectAction;
 import org.apache.isis.core.metamodel.spec.feature.ObjectAssociation;
 import org.apache.isis.core.metamodel.spec.feature.OneToOneAssociation;
-import org.apache.isis.core.runtime.sessiontemplate.AbstractIsisSessionTemplate;
+import org.apache.isis.core.metamodel.specloader.SpecificationLoader;
+import org.apache.isis.core.runtime.system.context.IsisContext;
+import org.apache.isis.core.runtime.system.persistence.PersistenceSession;
+import org.apache.isis.core.runtime.system.session.IsisSessionFactory;
 import org.apache.isis.core.runtime.system.transaction.IsisTransactionManager;
-import org.apache.isis.core.runtime.system.transaction.TransactionalClosure;
-import org.apache.isis.core.runtime.system.transaction.TransactionalClosureWithReturn;
 import org.apache.isis.schema.cmd.v1.ActionDto;
 import org.apache.isis.schema.cmd.v1.CommandDto;
 import org.apache.isis.schema.cmd.v1.MemberDto;
 import org.apache.isis.schema.cmd.v1.ParamDto;
 import org.apache.isis.schema.cmd.v1.ParamsDto;
 import org.apache.isis.schema.cmd.v1.PropertyDto;
+import org.apache.isis.schema.common.v1.CollectionDto;
 import org.apache.isis.schema.common.v1.InteractionType;
 import org.apache.isis.schema.common.v1.OidDto;
 import org.apache.isis.schema.common.v1.OidsDto;
+import org.apache.isis.schema.common.v1.ValueDto;
+import org.apache.isis.schema.common.v1.ValueType;
 import org.apache.isis.schema.common.v1.ValueWithTypeDto;
 import org.apache.isis.schema.utils.CommandDtoUtils;
 import org.apache.isis.schema.utils.CommonDtoUtils;
 
-/**
- */
-public abstract class CommandExecutionAbstract extends AbstractIsisSessionTemplate {
-
-    private final static Logger LOG = LoggerFactory.getLogger(CommandExecutionAbstract.class);
-
-    public enum SudoPolicy {
-        /**
-         * For example, regular background commands.
-         */
-        NO_SWITCH,
-        /**
-         * For example, replayable commands.
-         */
-        SWITCH,
-    }
-
-    private final SudoPolicy sudoPolicy;
-
-    protected CommandExecutionAbstract(final SudoPolicy sudoPolicy) {
-        this.sudoPolicy = sudoPolicy;
-    }
-
-    // //////////////////////////////////////
-
-    /**
-     * Executes the command within a transaction, and with respect to the specified {@link SudoPolicy}
-     * specified in the constructor.
-     * 
-     * <p>
-     *     Intended to be called from an override of {@link #doExecute(Object)},
-     *     not from {@link #doExecuteWithTransaction(Object)} (because the error handling is different).
-     * </p>
-     *
-     * <p>
-     *     Subclasses can add additional policies (eg adjusting the clock) by overriding {@link #executeCommand(IsisTransactionManager, Command)} and delegating to {@link #doExecuteCommand(IsisTransactionManager, Command)} as required.
-     * </p>
-     *
-     * @return - any exception arising.  We don't throw this exception, because an exception might be what is expected (eg if replaying a command that itself failed to execute).
-     */
-    protected final Exception execute(
-            final IsisTransactionManager transactionManager,
-            final Command command) {
+@DomainService(nature = NatureOfService.DOMAIN)
+public class CommandExecutorServiceDefault implements CommandExecutorService {
 
-        try {
-            return executeCommandWithinTran(transactionManager, command);
-        } catch (final RuntimeException ex) {
-
-            // attempting to commit the xactn itself could cause an issue
-            // so we need extra exception handling here, it seems.
-
-            org.apache.isis.applib.annotation.Command.ExecuteIn executeIn = command.getExecuteIn();
-            LOG.warn("Exception when committing for: {} {}", executeIn, command.getMemberIdentifier(), ex);
-
-            //
-            // the previous transaction will have been aborted as part of the recovery handling within
-            // TransactionManager's executeCommandWithinTran
-            //
-            // we therefore start a new xactn in order to make sure that this background command is marked as a failure
-            // so it won't be attempted again.
-            //
-            transactionManager.executeWithinTransaction(new TransactionalClosure(){
-                @Override
-                public void execute() {
-
-                    final Interaction backgroundInteraction = interactionContext.getInteraction();
-                    final Interaction.Execution currentExecution = backgroundInteraction.getCurrentExecution();
-                    command.setStartedAt(
-                            currentExecution != null
-                                    ? currentExecution.getStartedAt()
-                                    : clockService.nowAsJavaSqlTimestamp());
-
-                    command.setCompletedAt(clockService.nowAsJavaSqlTimestamp());
-                    command.setException(Throwables.getStackTraceAsString(ex));
-                }
-            });
-
-            return ex;
-        }
-    }
+    private final static Logger LOG = LoggerFactory.getLogger(CommandExecutorServiceDefault.class);
 
-    private Exception executeCommandWithinTran(
-            final IsisTransactionManager transactionManager,
+    @Programmatic
+    public void executeCommand(
+            final CommandExecutorService.SudoPolicy sudoPolicy,
             final Command command) {
 
-        return transactionManager.executeWithinTransaction(
-                command,
-                new TransactionalClosureWithReturn<Exception>() {
-            @Override
-            public Exception execute() {
-
-                return executeCommandPerSudoPolicy(transactionManager, command);
-            }
-        });
-    }
-
-    private Exception executeCommandPerSudoPolicy(
-            final IsisTransactionManager transactionManager,
-            final Command command) {
+        // make sure that the service has been called with a 'in progress' transaction already set up
+        ensureTransactionInProgress();
 
         switch (sudoPolicy) {
         case NO_SWITCH:
-            return executeCommand(transactionManager, command);
+            executeCommand(command);
+            break;
         case SWITCH:
             final String user = command.getUser();
-            return sudoService.sudo(user, new Callable<Exception>() {
+            sudoService.sudo(user, new Runnable() {
                 @Override
-                public Exception call() {
-                    return executeCommand(transactionManager, command);
+                public void run() {
+                    executeCommand(command);
                 }
             });
+            break;
         default:
-            throw new IllegalStateException("Unrecognized sudoPolicy: " + sudoPolicy);
+            throw new IllegalStateException("Probable framework error, unrecognized sudoPolicy: " + sudoPolicy);
         }
+
+        // double check that we've ended up in the same state
+        ensureTransactionInProgress();
     }
 
-    /**
-     * Simply delegates to {@link #doExecuteCommand(IsisTransactionManager, Command)}.
-     *
-     * Overridable, so execution policy can be adjusted if required,
-     * eg when replaying, adjust the clock before executing the command.
-     *
-     * @return - any exception arising
-     */
-    protected Exception executeCommand(
-            final IsisTransactionManager transactionManager,
-            final Command command) {
-        return doExecuteCommand(transactionManager, command);
+    private void ensureTransactionInProgress() {
+        final Transaction2 currentTransaction = transactionService.currentTransaction();
+        if(currentTransaction == null) {
+            throw new IllegalStateException("No current transaction");
+        }
+        final TransactionState transactionState = currentTransaction.getTransactionState();
+        if(!transactionState.canCommit()) {
+            throw new IllegalStateException("Current transaction is not in a state to be committed, is: " + transactionState);
+        }
     }
 
-    /**
-     * Not overrideable, but intended to be called by
-     * {@link #executeCommand(IsisTransactionManager, Command)} (which is).
-     *
-     * @return - any exception arising.  We don't throw this exception, because an exception might be
-     *           what is expected (eg if replaying a command that itself failed to execute).
-     */
-    protected final Exception doExecuteCommand(
-            final IsisTransactionManager transactionManager,
-            final Command command) {
+    protected void executeCommand(final Command command) {
 
         // setup for us by IsisTransactionManager; will have the transactionId of the backgroundCommand
-        final Interaction backgroundInteraction = interactionContext.getInteraction();
+        final Interaction interaction = interactionContext.getInteraction();
 
         final String memento = command.getMemento();
 
@@ -217,14 +131,14 @@ public abstract class CommandExecutionAbstract extends AbstractIsisSessionTempla
         RuntimeException exceptionIfAny = null;
 
         try {
-            command.setExecutor(Executor.BACKGROUND);
+            command.setExecutor(Command.Executor.BACKGROUND);
 
             // responsibility for setting the Command#startedAt is in the ActionInvocationFacet or
             // PropertySetterFacet, but this is run if the domain object was found.  If the domain object is
             // thrown then we would have a command with only completedAt, which is inconsistent.
             // Therefore instead we copy down from the backgroundInteraction (similar to how we populate the
             // completedAt at the end)
-            final Interaction.Execution priorExecution = backgroundInteraction.getPriorExecution();
+            final Interaction.Execution priorExecution = interaction.getPriorExecution();
 
             final Timestamp startedAt = priorExecution != null
                     ? priorExecution.getStartedAt()
@@ -270,8 +184,6 @@ public abstract class CommandExecutionAbstract extends AbstractIsisSessionTempla
                     //
 
                     // REVIEW: this doesn't really make sense if >1 action
-                    // in any case, the capturing of the action interaction should be the
-                    // responsibility of auditing/profiling
                     if(resultAdapter != null) {
                         Bookmark resultBookmark = CommandUtil.bookmarkFor(resultAdapter);
                         command.setResult(resultBookmark);
@@ -293,43 +205,58 @@ public abstract class CommandExecutionAbstract extends AbstractIsisSessionTempla
                     final ObjectAdapter newValueAdapter = newValueAdapterFor(propertyDto);
 
                     property.set(targetAdapter, newValueAdapter, InteractionInitiatedBy.FRAMEWORK);
+
                     // there is no return value for property modifications.
                 }
             }
 
         } catch (RuntimeException ex) {
 
-            LOG.warn("Exception for: {} {}", executeIn, command.getMemberIdentifier(), ex);
+            LOG.warn("Exception when executing : {} {}", executeIn, command.getMemberIdentifier(), ex);
 
-            // hmmm, this doesn't really make sense if >1 action
-            //
-            // in any case, the capturing of the result of the action invocation should be the
-            // responsibility of the interaction...
-            command.setException(Throwables.getStackTraceAsString(ex));
+            exceptionIfAny = ex;
+        }
 
-            // lower down the stack the IsisTransactionManager will have set the transaction to abort
-            // however, we don't want that to occur (because any changes made to the backgroundCommand itself
-            // would also be rolled back, and it would keep getting picked up again by a scheduler for
-            // processing); instead we clear the abort cause and ensure we can continue.
-            transactionManager.getCurrentTransaction().clearAbortCauseAndContinue();
+        // committing the xactn might also trigger an exception
+        try {
+            transactionService.nextTransaction(TransactionService3.Policy.ALWAYS);
+        } catch(RuntimeException ex) {
 
-            // checked at the end
-            exceptionIfAny = ex;
+            LOG.warn("Exception when committing : {} {}", executeIn, command.getMemberIdentifier(), ex);
 
+            if(exceptionIfAny == null) {
+                exceptionIfAny = ex;
+            }
+
+            // this will set up a new transaction
+            transactionService.nextTransaction();
         }
 
         // it's possible that there is no priorExecution, specifically if there was an exception
-        // invoking the action.  We therefore need to guard that case.
-        final Interaction.Execution priorExecution = backgroundInteraction.getPriorExecution();
+        // when performing the action invocation/property edit.  We therefore need to guard that case.
+        final Interaction.Execution priorExecution = interaction.getPriorExecution();
+        if (command.getStartedAt() == null) {
+            // if attempting to commit the xactn threw an error, we will (I think?) have lost this info, so need to
+            // capture
+            command.setStartedAt(
+                    priorExecution != null
+                            ? priorExecution.getStartedAt()
+                            : clockService.nowAsJavaSqlTimestamp());
+        }
+
         final Timestamp completedAt =
                 priorExecution != null
                         ? priorExecution.getCompletedAt()
                         : clockService.nowAsJavaSqlTimestamp();  // close enough...
         command.setCompletedAt(completedAt);
 
-        return exceptionIfAny;
+        if(exceptionIfAny != null) {
+            command.setException(Throwables.getStackTraceAsString(exceptionIfAny));
+        }
     }
 
+    // //////////////////////////////////////
+
     private static ObjectAction findObjectAction(
             final ObjectAdapter targetAdapter,
             final String actionId) throws RuntimeException {
@@ -380,38 +307,13 @@ public abstract class CommandExecutionAbstract extends AbstractIsisSessionTempla
         final List<ObjectAssociation> associations = specification.getAssociations(Contributed.INCLUDED);
         for (final ObjectAssociation association : associations) {
             if( association.getIdentifier().toClassAndNameIdentityString().equals(propertyId) &&
-                association instanceof OneToOneAssociation) {
+                    association instanceof OneToOneAssociation) {
                 return (OneToOneAssociation) association;
             }
         }
         return null;
     }
 
-    private ObjectAdapter[] argAdaptersFor(final ActionInvocationMemento aim)  {
-        final int numArgs = aim.getNumArgs();
-        final List<ObjectAdapter> argumentAdapters = Lists.newArrayList();
-        for(int i=0; i<numArgs; i++) {
-            final ObjectAdapter argAdapter = argAdapterFor(aim, i);
-            argumentAdapters.add(argAdapter);
-        }
-        return argumentAdapters.toArray(new ObjectAdapter[]{});
-    }
-
-    private ObjectAdapter argAdapterFor(final ActionInvocationMemento aim, int num) {
-        final Class<?> argType;
-        try {
-            argType = aim.getArgType(num);
-            final Object arg = aim.getArg(num, argType);
-            if(arg == null) {
-                return null;
-            }
-            return adapterFor(arg);
-
-        } catch (ClassNotFoundException e) {
-            throw new RuntimeException(e);
-        }
-    }
-
     private ObjectAdapter[] argAdaptersFor(final ActionDto actionDto) {
         final List<ParamDto> params = paramDtosFrom(actionDto);
         final List<ObjectAdapter> args = Lists.newArrayList(
@@ -437,6 +339,65 @@ public abstract class CommandExecutionAbstract extends AbstractIsisSessionTempla
         return Collections.emptyList();
     }
 
+    private ObjectAdapter adapterFor(final Object targetObject) {
+        if(targetObject instanceof OidDto) {
+            final OidDto oidDto = (OidDto) targetObject;
+            return adapterFor(oidDto);
+        }
+        if(targetObject instanceof CollectionDto) {
+            final CollectionDto collectionDto = (CollectionDto) targetObject;
+            final List<ValueDto> valueDtoList = collectionDto.getValue();
+            final List<Object> pojoList = Lists.newArrayList();
+            for (final ValueDto valueDto : valueDtoList) {
+                ValueType valueType = collectionDto.getType();
+                final Object valueOrOidDto = CommonDtoUtils.getValue(valueDto, valueType);
+                // converting from adapter and back means we handle both
+                // collections of references and of values
+                final ObjectAdapter objectAdapter = adapterFor(valueOrOidDto);
+                Object pojo = objectAdapter != null ? objectAdapter.getObject() : null;
+                pojoList.add(pojo);
+            }
+            return adapterFor(pojoList);
+        }
+        if(targetObject instanceof Bookmark) {
+            final Bookmark bookmark = (Bookmark) targetObject;
+            return adapterFor(bookmark);
+        }
+        return getPersistenceSession().adapterFor(targetObject);
+    }
+
+    private ObjectAdapter adapterFor(final OidDto oidDto) {
+        final Bookmark bookmark = Bookmark.from(oidDto);
+        return adapterFor(bookmark);
+    }
+
+    private ObjectAdapter adapterFor(final Bookmark bookmark) {
+        final RootOid rootOid = RootOid.create(bookmark);
+        return adapterFor(rootOid);
+    }
+
+    private ObjectAdapter adapterFor(final RootOid rootOid) {
+        return getPersistenceSession().adapterFor(rootOid);
+    }
+
+    // //////////////////////////////////////
+
+    protected IsisSessionFactory getIsisSessionFactory() {
+        return IsisContext.getSessionFactory();
+    }
+
+    protected PersistenceSession getPersistenceSession() {
+        return getIsisSessionFactory().getCurrentSession().getPersistenceSession();
+    }
+
+    protected IsisTransactionManager getTransactionManager(PersistenceSession persistenceSession) {
+        return persistenceSession.getTransactionManager();
+    }
+
+    protected SpecificationLoader getSpecificationLoader() {
+        return getIsisSessionFactory().getSpecificationLoader();
+    }
+
     // //////////////////////////////////////
 
     @javax.inject.Inject
@@ -454,4 +415,7 @@ public abstract class CommandExecutionAbstract extends AbstractIsisSessionTempla
     @javax.inject.Inject
     ClockService clockService;
 
+    @javax.inject.Inject
+    TransactionService3 transactionService;
+
 }
diff --git a/core/viewer-wicket-impl/src/main/java/org/apache/isis/viewer/wicket/viewer/services/Object_clearHints.java b/core/viewer-wicket-impl/src/main/java/org/apache/isis/viewer/wicket/viewer/services/Object_clearHints.java
index 97e9f05..24adfe1 100644
--- a/core/viewer-wicket-impl/src/main/java/org/apache/isis/viewer/wicket/viewer/services/Object_clearHints.java
+++ b/core/viewer-wicket-impl/src/main/java/org/apache/isis/viewer/wicket/viewer/services/Object_clearHints.java
@@ -20,6 +20,7 @@ package org.apache.isis.viewer.wicket.viewer.services;
 
 import org.apache.isis.applib.annotation.Action;
 import org.apache.isis.applib.annotation.ActionLayout;
+import org.apache.isis.applib.annotation.CommandReification;
 import org.apache.isis.applib.annotation.Contributed;
 import org.apache.isis.applib.annotation.MemberOrder;
 import org.apache.isis.applib.annotation.Mixin;
@@ -41,7 +42,8 @@ public class Object_clearHints {
 
     @Action(
             domainEvent = ActionDomainEvent.class,
-            semantics = SemanticsOf.IDEMPOTENT
+            semantics = SemanticsOf.IDEMPOTENT,
+            command = CommandReification.DISABLED
     )
     @ActionLayout(
             contributed = Contributed.AS_ACTION,

-- 
To stop receiving notification emails like this one, please contact
danhaywood@apache.org.