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/01/31 18:47:30 UTC

[isis] branch maint-1.16.1 updated: ISIS-1569: continues if replicated an exception; utility methods for DTOs

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


The following commit(s) were added to refs/heads/maint-1.16.1 by this push:
     new f0a952d  ISIS-1569: continues if replicated an exception; utility methods for DTOs
f0a952d is described below

commit f0a952dc2effa52d57e3b3c7757f4494040c179e
Author: Dan Haywood <da...@haywood-associates.co.uk>
AuthorDate: Wed Jan 31 18:34:53 2018 +0000

    ISIS-1569: continues if replicated an exception; utility methods for DTOs
    
    ContentMappingServiceForCommandDto now takes responsibility for automatically copying over 5 fields from the CommandWithDto (entity) into the user data of the CommandDto.
    One of these is the exception, if any.  This is used by BackgroundCommandExecution to CONTINUE to process EVEN IF an exception occurs on the slave, if an exception also had occurred on the master.
---
 .../conmap/ContentMappingServiceForCommandDto.java | 46 ++++++++++++++++----
 .../applib/services/command/CommandWithDto.java    |  6 +++
 .../apache/isis/schema/utils/CommandDtoUtils.java  | 36 ++++++++++++++++
 .../apache/isis/schema/utils/CommonDtoUtils.java   | 35 +++++++++++++++
 .../isis/schema/utils/CommandDtoUtils_Test.java    | 50 ++++++++++++++++++++++
 ...nDtoUtilsTest.java => CommonDtoUtils_Test.java} | 37 ++++++++++++++--
 .../background/BackgroundCommandExecution.java     | 34 ++++++++++++---
 7 files changed, 228 insertions(+), 16 deletions(-)

diff --git a/core/applib/src/main/java/org/apache/isis/applib/conmap/ContentMappingServiceForCommandDto.java b/core/applib/src/main/java/org/apache/isis/applib/conmap/ContentMappingServiceForCommandDto.java
index fdec5da..a11b9d3 100644
--- a/core/applib/src/main/java/org/apache/isis/applib/conmap/ContentMappingServiceForCommandDto.java
+++ b/core/applib/src/main/java/org/apache/isis/applib/conmap/ContentMappingServiceForCommandDto.java
@@ -27,10 +27,13 @@ import javax.ws.rs.core.MediaType;
 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.command.CommandDtoProcessor;
 import org.apache.isis.applib.services.command.CommandWithDto;
 import org.apache.isis.applib.services.metamodel.MetaModelService5;
 import org.apache.isis.schema.cmd.v1.CommandDto;
+import org.apache.isis.schema.common.v1.PeriodDto;
+import org.apache.isis.schema.utils.CommandDtoUtils;
 import org.apache.isis.schema.utils.jaxbadapters.JavaSqlTimestampXmlGregorianCalendarAdapter;
 
 @DomainService(
@@ -72,14 +75,9 @@ public class ContentMappingServiceForCommandDto implements ContentMappingService
             CommandWithDto commandWithDto,
             final MetaModelService5 metaModelService) {
         final CommandDto commandDto = commandWithDto.asDto();
-        /**
-         * the timestamp field was only introduced in v1.4 of cmd.xsd, so there's no guarantee it will have been
-         * populated.  We therefore copy the value in from CommandWithDto entity.
-         */
-        if(commandDto.getTimestamp() == null) {
-            final Timestamp timestamp = commandWithDto.getTimestamp();
-            commandDto.setTimestamp(JavaSqlTimestampXmlGregorianCalendarAdapter.print(timestamp));
-        }
+
+        copyOver(commandWithDto, commandDto);
+
         final CommandDtoProcessor commandDtoProcessor =
                 metaModelService.commandDtoProcessorFor(commandDto.getMember().getLogicalMemberIdentifier());
         if (commandDtoProcessor == null) {
@@ -88,6 +86,38 @@ public class ContentMappingServiceForCommandDto implements ContentMappingService
         return commandDtoProcessor.process(commandWithDto, commandDto);
     }
 
+    private static void copyOver(final CommandWithDto commandWithDto, final CommandDto commandDto) {
+
+        // for some reason this isn't being persisted initially, so patch it in.  TODO: should fix this
+        commandDto.setUser(commandWithDto.getUser());
+
+        // the timestamp field was only introduced in v1.4 of cmd.xsd, so there's no guarantee
+        // it will have been populated.  We therefore copy the value in from CommandWithDto entity.
+        if(commandDto.getTimestamp() == null) {
+            final Timestamp timestamp = commandWithDto.getTimestamp();
+            commandDto.setTimestamp(JavaSqlTimestampXmlGregorianCalendarAdapter.print(timestamp));
+        }
+
+        CommandDtoUtils.setUserData(commandDto,
+                CommandWithDto.USERDATA_KEY_TARGET_CLASS, commandWithDto.getTargetClass());
+        CommandDtoUtils.setUserData(commandDto,
+                CommandWithDto.USERDATA_KEY_TARGET_ACTION, commandWithDto.getTargetAction());
+        CommandDtoUtils.setUserData(commandDto,
+                CommandWithDto.USERDATA_KEY_ARGUMENTS, commandWithDto.getArguments());
+
+        final Bookmark result = commandWithDto.getResult();
+        CommandDtoUtils.setUserData(commandDto,
+                CommandWithDto.USERDATA_KEY_RETURN_VALUE, result != null ? result.toString() : null);
+        // knowing whether there was an exception is on the master is used to determine whether to
+        // continue when replayed on the slave if an exception occurs there also
+        CommandDtoUtils.setUserData(commandDto,
+                CommandWithDto.USERDATA_KEY_EXCEPTION, commandWithDto.getException());
+
+        PeriodDto timings = CommandDtoUtils.timingsFor(commandDto);
+        timings.setStartedAt(JavaSqlTimestampXmlGregorianCalendarAdapter.print(commandWithDto.getStartedAt()));
+        timings.setCompletedAt(JavaSqlTimestampXmlGregorianCalendarAdapter.print(commandWithDto.getCompletedAt()));
+    }
+
     @Inject
     MetaModelService5 metaModelService;
 
diff --git a/core/applib/src/main/java/org/apache/isis/applib/services/command/CommandWithDto.java b/core/applib/src/main/java/org/apache/isis/applib/services/command/CommandWithDto.java
index e0e1b60..153b75d 100644
--- a/core/applib/src/main/java/org/apache/isis/applib/services/command/CommandWithDto.java
+++ b/core/applib/src/main/java/org/apache/isis/applib/services/command/CommandWithDto.java
@@ -21,6 +21,12 @@ import org.apache.isis.schema.cmd.v1.CommandDto;
 
 public interface CommandWithDto extends Command {
 
+    String USERDATA_KEY_TARGET_CLASS = "targetClass";
+    String USERDATA_KEY_TARGET_ACTION = "targetAction";
+    String USERDATA_KEY_ARGUMENTS = "arguments";
+    String USERDATA_KEY_RETURN_VALUE = "returnValue";
+    String USERDATA_KEY_EXCEPTION = "exception";
+
     @Programmatic
     CommandDto asDto();
 }
diff --git a/core/applib/src/main/java/org/apache/isis/schema/utils/CommandDtoUtils.java b/core/applib/src/main/java/org/apache/isis/schema/utils/CommandDtoUtils.java
index f3a3b7a..f61260c 100644
--- a/core/applib/src/main/java/org/apache/isis/schema/utils/CommandDtoUtils.java
+++ b/core/applib/src/main/java/org/apache/isis/schema/utils/CommandDtoUtils.java
@@ -35,8 +35,10 @@ import com.google.common.io.Resources;
 
 import org.apache.isis.schema.cmd.v1.ActionDto;
 import org.apache.isis.schema.cmd.v1.CommandDto;
+import org.apache.isis.schema.cmd.v1.MapDto;
 import org.apache.isis.schema.cmd.v1.ParamsDto;
 import org.apache.isis.schema.common.v1.OidsDto;
+import org.apache.isis.schema.common.v1.PeriodDto;
 
 public final class CommandDtoUtils {
 
@@ -115,4 +117,38 @@ public final class CommandDtoUtils {
         return parameters;
     }
 
+    public static PeriodDto timingsFor(final CommandDto commandDto) {
+        PeriodDto timings = commandDto.getTimings();
+        if(timings == null) {
+            timings = new PeriodDto();
+            commandDto.setTimings(timings);
+        }
+        return timings;
+    }
+
+    public static String getUserData(final CommandDto dto, final String key) {
+        if(dto == null || key == null) {
+            return null;
+        }
+        return CommonDtoUtils.getMapValue(dto.getUserData(), key);
+    }
+
+    public static void setUserData(
+            final CommandDto dto, final String key, final String value) {
+        if(dto == null || key == null) {
+            return;
+        }
+        final MapDto userData = userDataFor(dto);
+        CommonDtoUtils.putMapKeyValue(userData, key, value);
+    }
+
+    private static MapDto userDataFor(final CommandDto commandDto) {
+        MapDto userData = commandDto.getUserData();
+        if(userData == null) {
+            userData = new MapDto();
+            commandDto.setUserData(userData);
+        }
+        return userData;
+    }
+
 }
diff --git a/core/applib/src/main/java/org/apache/isis/schema/utils/CommonDtoUtils.java b/core/applib/src/main/java/org/apache/isis/schema/utils/CommonDtoUtils.java
index bba378f..6c3ea91 100644
--- a/core/applib/src/main/java/org/apache/isis/schema/utils/CommonDtoUtils.java
+++ b/core/applib/src/main/java/org/apache/isis/schema/utils/CommonDtoUtils.java
@@ -23,7 +23,11 @@ import java.math.BigInteger;
 import java.util.Collection;
 
 import com.google.common.base.Function;
+import com.google.common.base.Objects;
+import com.google.common.base.Optional;
+import com.google.common.base.Predicate;
 import com.google.common.base.Strings;
+import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableMap;
 
 import org.joda.time.DateTime;
@@ -35,6 +39,7 @@ import org.apache.isis.applib.services.bookmark.Bookmark;
 import org.apache.isis.applib.services.bookmark.BookmarkService;
 import org.apache.isis.applib.value.Blob;
 import org.apache.isis.applib.value.Clob;
+import org.apache.isis.schema.cmd.v1.MapDto;
 import org.apache.isis.schema.cmd.v1.ParamDto;
 import org.apache.isis.schema.common.v1.BlobDto;
 import org.apache.isis.schema.common.v1.ClobDto;
@@ -462,7 +467,37 @@ public final class CommonDtoUtils {
 
     //endregion
 
+    public static String getMapValue(final MapDto mapDto, final String key) {
+        if(mapDto == null) {
+            return null;
+        }
+        final Optional<MapDto.Entry> entryIfAny = entryIfAnyFor(mapDto, key);
+        return entryIfAny.isPresent() ? entryIfAny.get().getValue() : null;
+    }
 
+    public static void putMapKeyValue(final MapDto mapDto, final String key, final String value) {
+        if(mapDto == null) {
+            return;
+        }
+        final Optional<MapDto.Entry> entryIfAny = entryIfAnyFor(mapDto, key);
+        if(entryIfAny.isPresent()) {
+            entryIfAny.get().setValue(value);
+        } else {
+            final MapDto.Entry entry = new MapDto.Entry();
+            entry.setKey(key);
+            entry.setValue(value);
+            mapDto.getEntry().add(entry);
+        }
+    }
 
+    private static Optional<MapDto.Entry> entryIfAnyFor(final MapDto mapDto, final String key) {
+        return FluentIterable.from(mapDto.getEntry())
+                    .firstMatch(new Predicate<MapDto.Entry>() {
+                        @Override
+                        public boolean apply(final MapDto.Entry entry) {
+                            return Objects.equal(entry.getKey(), key);
+                        }
+                    });
+    }
 
 }
diff --git a/core/applib/src/test/java/org/apache/isis/schema/utils/CommandDtoUtils_Test.java b/core/applib/src/test/java/org/apache/isis/schema/utils/CommandDtoUtils_Test.java
new file mode 100644
index 0000000..cd4b870
--- /dev/null
+++ b/core/applib/src/test/java/org/apache/isis/schema/utils/CommandDtoUtils_Test.java
@@ -0,0 +1,50 @@
+package org.apache.isis.schema.utils;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.isis.schema.cmd.v1.CommandDto;
+import org.apache.isis.schema.cmd.v1.MapDto;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.junit.Assert.assertThat;
+
+public class CommandDtoUtils_Test {
+
+    CommandDto dto;
+    @Before
+    public void setUp() throws Exception {
+        dto = new CommandDto();
+    }
+
+    @Test
+    public void getUserData() {
+
+        // null is just ignored
+        assertThat(CommandDtoUtils.getUserData(null, "someKey"), is(nullValue()));
+
+        // empty
+        assertThat(CommandDtoUtils.getUserData(dto, "someKey"), is(nullValue()));
+
+        // populated
+        final MapDto mapDto = new MapDto();
+        CommonDtoUtils.putMapKeyValue(mapDto, "someKey", "someValue");
+        dto.setUserData(mapDto);
+
+        assertThat(CommandDtoUtils.getUserData(dto, "someKey"), is("someValue"));
+    }
+
+    @Test
+    public void setUserData() {
+
+        CommandDtoUtils.setUserData(dto, "someKey", "someValue");
+        assertThat(CommandDtoUtils.getUserData(dto, "someKey"), is("someValue"));
+
+        CommandDtoUtils.setUserData(dto, "someKey", "someOtherValue");
+        assertThat(CommandDtoUtils.getUserData(dto, "someKey"), is("someOtherValue"));
+
+        CommandDtoUtils.setUserData(dto, "someKey", null);
+        assertThat(CommandDtoUtils.getUserData(dto, "someKey"), is(nullValue()));
+    }
+}
\ No newline at end of file
diff --git a/core/applib/src/test/java/org/apache/isis/schema/utils/CommonDtoUtilsTest.java b/core/applib/src/test/java/org/apache/isis/schema/utils/CommonDtoUtils_Test.java
similarity index 64%
rename from core/applib/src/test/java/org/apache/isis/schema/utils/CommonDtoUtilsTest.java
rename to core/applib/src/test/java/org/apache/isis/schema/utils/CommonDtoUtils_Test.java
index b07dc43..e6c1eb7 100644
--- a/core/applib/src/test/java/org/apache/isis/schema/utils/CommonDtoUtilsTest.java
+++ b/core/applib/src/test/java/org/apache/isis/schema/utils/CommonDtoUtils_Test.java
@@ -25,14 +25,16 @@ import org.junit.Test;
 
 import org.apache.isis.applib.services.bookmark.BookmarkService;
 import org.apache.isis.core.unittestsupport.jmocking.JUnitRuleMockery2;
+import org.apache.isis.schema.cmd.v1.MapDto;
 import org.apache.isis.schema.common.v1.ValueDto;
 import org.apache.isis.schema.common.v1.ValueType;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.CoreMatchers.nullValue;
 import static org.junit.Assert.assertThat;
 
-public class CommonDtoUtilsTest {
+public class CommonDtoUtils_Test {
 
     @Rule
     public JUnitRuleMockery2 context = JUnitRuleMockery2.createFor(JUnitRuleMockery2.Mode.INTERFACES_AND_CLASSES);
@@ -41,7 +43,7 @@ public class CommonDtoUtilsTest {
     private BookmarkService mockBookmarkService;
 
     @Test
-    public void enums() throws Exception {
+    public void enums() {
         test(Vertical.DOWN);
     }
 
@@ -50,7 +52,7 @@ public class CommonDtoUtilsTest {
     }
 
     @Test
-    public void nested_enums() throws Exception {
+    public void nested_enums() {
         test(Horizontal.LEFT);
     }
 
@@ -72,4 +74,33 @@ public class CommonDtoUtilsTest {
         Assert.assertEquals(value, enumVal);
     }
 
+    @Test
+    public void getMapValue() {
+        Assert.assertThat(CommonDtoUtils.getMapValue(null, "someKey"), is(nullValue()));
+        Assert.assertThat(CommonDtoUtils.getMapValue(new MapDto(), "someKey"), is(nullValue()));
+
+        // given
+        final MapDto mapDto = new MapDto();
+        final MapDto.Entry e = new MapDto.Entry();
+        e.setKey("someKey");
+        e.setValue("someValue");
+        mapDto.getEntry().add(e);
+
+        Assert.assertThat(CommonDtoUtils.getMapValue(mapDto, "someKey"), is("someValue"));
+        Assert.assertThat(CommonDtoUtils.getMapValue(mapDto, "someThingElse"), is(nullValue()));
+    }
+
+    @Test
+    public void putMapKeyValue() {
+
+        // is ignored
+        CommonDtoUtils.putMapKeyValue(null, "someKey", "someValue");
+
+        // when
+        final MapDto mapDto = new MapDto();
+        CommonDtoUtils.putMapKeyValue(mapDto, "someKey", "someValue");
+
+        Assert.assertThat(CommonDtoUtils.getMapValue(mapDto, "someKey"), is("someValue"));
+    }
+
 }
\ No newline at end of file
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 07d2e67..7f616aa 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
@@ -35,6 +35,7 @@ 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.command.CommandWithDto;
 import org.apache.isis.applib.services.iactn.Interaction;
 import org.apache.isis.applib.services.iactn.InteractionContext;
 import org.apache.isis.applib.services.jaxb.JaxbService;
@@ -84,8 +85,12 @@ public abstract class BackgroundCommandExecution extends AbstractIsisSessionTemp
         CONTINUE,
         /**
          * For example, replayable commands.
+         *
+         * To be precise, replay will quit only if there is an exception on the slave where there
+         * was none on the master.  Put another way, a replicated command that failed on the master
+         * will not cause the slave to stop executing if it caused an exception.
          */
-        QUIT
+        QUIT,
     }
 
     public enum SudoPolicy {
@@ -260,6 +265,8 @@ public abstract class BackgroundCommandExecution extends AbstractIsisSessionTemp
         LOG.info("Executing: {} {}", executeIn, command.getMemberIdentifier());
 
         RuntimeException exceptionIfAny = null;
+        String origExceptionIfAny = null;
+
         try {
             command.setExecutor(Executor.BACKGROUND);
 
@@ -320,6 +327,10 @@ public abstract class BackgroundCommandExecution extends AbstractIsisSessionTemp
 
                 final CommandDto dto = jaxbService.fromXml(CommandDto.class, memento);
 
+                // if the command being executed was replayed, then in its userData it will holds
+                // details of any exception that might have occurred.
+                origExceptionIfAny = CommandDtoUtils.getUserData(dto, CommandWithDto.USERDATA_KEY_EXCEPTION);
+
                 final MemberDto memberDto = dto.getMember();
                 final String memberId = memberDto.getMemberIdentifier();
 
@@ -398,6 +409,7 @@ public abstract class BackgroundCommandExecution extends AbstractIsisSessionTemp
 
             // checked at the end
             exceptionIfAny = ex;
+
         }
 
         // it's possible that there is no priorExecution, specifically if there was an exception
@@ -409,8 +421,8 @@ public abstract class BackgroundCommandExecution extends AbstractIsisSessionTemp
                         : clockService.nowAsJavaSqlTimestamp();  // close enough...
         command.setCompletedAt(completedAt);
 
-        // if we hit an exception processing this command, then quit if instructed
-        return determineIfContinue(exceptionIfAny);
+        // if we hit an exception processing this command but the master did not, then quit if instructed
+        return determineIfContinue(origExceptionIfAny, exceptionIfAny);
     }
 
     private static ObjectAction findObjectAction(
@@ -439,9 +451,21 @@ public abstract class BackgroundCommandExecution extends AbstractIsisSessionTemp
         return property;
     }
 
-    protected boolean determineIfContinue(final RuntimeException exceptionIfAny) {
+    private boolean determineIfContinue(
+            final String origExceptionIfAny,
+            final RuntimeException exceptionIfAny) {
+
+        if(origExceptionIfAny != null) {
+            return true;
+        }
+
+        // there was no exception on master, so do we continue?
+        return determineIfContinue(exceptionIfAny);
+    }
+
+    private boolean determineIfContinue(final RuntimeException exceptionIfAny) {
         final boolean shouldQuit = exceptionIfAny != null &&
-                      onExceptionPolicy == BackgroundCommandExecution.OnExceptionPolicy.QUIT;
+                      onExceptionPolicy == OnExceptionPolicy.QUIT;
         return !shouldQuit;
     }
 

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