Merge pull request #789 from CJSCommonPlatform/make-commands-fail-if-…

…in-progress Commands now fail if the command is already in progress
CJSCommonPlatform · Oct 31, 2019 · 00d7d0d · 00d7d0d
2 parents 3cc5173 + 149567f
commit 00d7d0d
Show file tree

Hide file tree

Showing 13 changed files with 195 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,8 @@ on [Keep a CHANGELOG](http://keepachangelog.com/). This project adheres to
 [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
+### Changed
+- Commands now fail with an UnrunnableSystemCommandException if the command is already in progress
 
 ## [6.2.2] - 2019-10-23
 ### Added

diff --git a/...x-api/src/main/java/uk/gov/justice/services/jmx/api/UnrunnableSystemCommandException.java b/...x-api/src/main/java/uk/gov/justice/services/jmx/api/UnrunnableSystemCommandException.java
@@ -0,0 +1,8 @@
+package uk.gov.justice.services.jmx.api;
+
+public class UnrunnableSystemCommandException extends RuntimeException {
+
+    public UnrunnableSystemCommandException(final String message) {
+        super(message);
+    }
+}
diff --git a/...-api/src/main/java/uk/gov/justice/services/jmx/api/UnsupportedSystemCommandException.java b/...-api/src/main/java/uk/gov/justice/services/jmx/api/UnsupportedSystemCommandException.java
diff --git a/...command-handling/src/main/java/uk/gov/justice/services/jmx/api/mbean/SystemCommander.java b/...command-handling/src/main/java/uk/gov/justice/services/jmx/api/mbean/SystemCommander.java
@@ -3,7 +3,7 @@
 import static java.lang.String.format;
 
 import uk.gov.justice.services.jmx.api.CommandNotFoundException;
-import uk.gov.justice.services.jmx.api.UnsupportedSystemCommandException;
+import uk.gov.justice.services.jmx.api.UnrunnableSystemCommandException;
 import uk.gov.justice.services.jmx.api.command.SystemCommand;
 import uk.gov.justice.services.jmx.api.domain.SystemCommandStatus;
 import uk.gov.justice.services.jmx.command.SystemCommandScanner;
@@ -36,11 +36,13 @@ public UUID call(final SystemCommand systemCommand) {
 
         logger.info(format("Received System Command '%s'", systemCommand.getName()));
 
-        if(asynchronousCommandRunnerBean.isSupported(systemCommand)) {
-            return asynchronousCommandRunnerBean.run(systemCommand);
-        } else {
-           throw new UnsupportedSystemCommandException(format("The system command '%s' is not supported on this context.", systemCommand.getName()));
+        if(asynchronousCommandRunnerBean.commandNotSupported(systemCommand)) {
+           throw new UnrunnableSystemCommandException(format("The system command '%s' is not supported on this context.", systemCommand.getName()));
+        } if(systemCommandStateBean.commandInProgress(systemCommand)) {
+           throw new UnrunnableSystemCommandException(format("Cannot run system command '%s'. A previous call to that command is still in progress.", systemCommand.getName()));
         }
+
+        return asynchronousCommandRunnerBean.run(systemCommand);
     }
 
     @Override

diff --git a/...-handling/src/main/java/uk/gov/justice/services/jmx/runner/AsynchronousCommandRunner.java b/...-handling/src/main/java/uk/gov/justice/services/jmx/runner/AsynchronousCommandRunner.java
@@ -45,8 +45,8 @@ public UUID run(final SystemCommand systemCommand) {
         return commandId;
     }
 
-    public boolean isSupported(final SystemCommand systemCommand) {
-        return systemCommandRunner.isSupported(systemCommand);
+    public boolean commandNotSupported(final SystemCommand systemCommand) {
+        return ! systemCommandRunner.isSupported(systemCommand);
     }
 
     private void fireCommandReceived(final UUID commandId, final SystemCommand systemCommand) {

diff --git a/...ommand-handling/src/main/java/uk/gov/justice/services/jmx/runner/SystemCommandRunner.java b/...ommand-handling/src/main/java/uk/gov/justice/services/jmx/runner/SystemCommandRunner.java
@@ -2,12 +2,19 @@
 
 import static java.lang.String.format;
 import static javax.transaction.Transactional.TxType.NEVER;
+import static javax.transaction.Transactional.TxType.REQUIRES_NEW;
+import static uk.gov.justice.services.jmx.api.domain.CommandState.COMMAND_IN_PROGRESS;
+import static uk.gov.justice.services.jmx.api.domain.CommandState.COMMAND_RECEIVED;
 
 import uk.gov.justice.services.framework.utilities.exceptions.StackTraceProvider;
 import uk.gov.justice.services.jmx.api.SystemCommandInvocationFailedException;
 import uk.gov.justice.services.jmx.api.command.SystemCommand;
+import uk.gov.justice.services.jmx.api.domain.CommandState;
+import uk.gov.justice.services.jmx.api.domain.SystemCommandStatus;
 import uk.gov.justice.services.jmx.command.SystemCommandStore;
+import uk.gov.justice.services.jmx.state.persistence.SystemCommandStatusRepository;
 
+import java.util.Optional;
 import java.util.UUID;
 
 import javax.inject.Inject;

diff --git a/...ing/src/main/java/uk/gov/justice/services/jmx/state/observers/SystemCommandStateBean.java b/...ing/src/main/java/uk/gov/justice/services/jmx/state/observers/SystemCommandStateBean.java
@@ -1,7 +1,11 @@
 package uk.gov.justice.services.jmx.state.observers;
 
 import static javax.transaction.Transactional.TxType.REQUIRES_NEW;
+import static uk.gov.justice.services.jmx.api.domain.CommandState.COMMAND_IN_PROGRESS;
+import static uk.gov.justice.services.jmx.api.domain.CommandState.COMMAND_RECEIVED;
 
+import uk.gov.justice.services.jmx.api.command.SystemCommand;
+import uk.gov.justice.services.jmx.api.domain.CommandState;
 import uk.gov.justice.services.jmx.api.domain.SystemCommandStatus;
 import uk.gov.justice.services.jmx.state.persistence.SystemCommandStatusRepository;
 
@@ -25,6 +29,19 @@ public void addSystemCommandState(final SystemCommandStatus systemCommandStatus)
 
     @Transactional(REQUIRES_NEW)
     public Optional<SystemCommandStatus> getCommandStatus(final UUID commandId) {
-        return systemCommandStatusRepository.findLatestStatus(commandId);
+        return systemCommandStatusRepository.findLatestStatusById(commandId);
+    }
+
+    @Transactional(REQUIRES_NEW)
+    public boolean commandInProgress(final SystemCommand systemCommand) {
+        final Optional<SystemCommandStatus> latestStatusByType = systemCommandStatusRepository.findLatestStatusByType(systemCommand);
+
+        if (latestStatusByType.isPresent()) {
+            final CommandState commandState = latestStatusByType.get().getCommandState();
+
+            return commandState == COMMAND_RECEIVED || commandState == COMMAND_IN_PROGRESS;
+        }
+
+        return false;
     }
 }
diff --git a/...ain/java/uk/gov/justice/services/jmx/state/persistence/SystemCommandStatusRepository.java b/...ain/java/uk/gov/justice/services/jmx/state/persistence/SystemCommandStatusRepository.java
@@ -8,6 +8,8 @@
 import static uk.gov.justice.services.jmx.api.domain.CommandState.valueOf;
 
 import uk.gov.justice.services.jdbc.persistence.SystemJdbcDataSourceProvider;
+import uk.gov.justice.services.jmx.api.command.SystemCommand;
+import uk.gov.justice.services.jmx.api.domain.CommandState;
 import uk.gov.justice.services.jmx.api.domain.SystemCommandStatus;
 
 import java.sql.Connection;
@@ -40,13 +42,19 @@ public class SystemCommandStatusRepository {
             "WHERE command_id = ? " +
             "ORDER BY status_changed_at";
 
-    private static final String FIND_LATEST = "SELECT " +
+    private static final String FIND_LATEST_BY_ID = "SELECT " +
             "command_id, command_name, command_state, status_changed_at, message " +
             "FROM system_command_status " +
             "WHERE command_id = ? " +
             "ORDER BY status_changed_at DESC " +
             "LIMIT 1";
 
+    private static final String FIND_LATEST_BY_TYPE = "SELECT " +
+            "command_id, command_name, command_state, status_changed_at, message " +
+            "FROM system_command_status " +
+            "WHERE command_name = ? " +
+            "ORDER BY status_changed_at DESC " +
+            "LIMIT 1";
 
     @Inject
     private SystemJdbcDataSourceProvider systemJdbcDataSourceProvider;
@@ -66,7 +74,7 @@ public void add(final SystemCommandStatus systemCommandStatus) {
 
             preparedStatement.executeUpdate();
 
-        } catch (SQLException e) {
+        } catch (final SQLException e) {
             throw new SystemCommandStatusPersistenceException("Failed to insert SystemCommandStatus", e);
         }
     }
@@ -85,7 +93,7 @@ public List<SystemCommandStatus> findAll() {
                 systemCommandStatuses.add(systemCommandStatus);
             }
 
-        } catch (SQLException e) {
+        } catch (final SQLException e) {
             throw new SystemCommandStatusPersistenceException("Failed to find all SystemCommandStatus", e);
         }
 
@@ -108,18 +116,18 @@ public List<SystemCommandStatus> findAllByCommandId(final UUID commandId) {
                 }
             }
 
-        } catch (SQLException e) {
+        } catch (final SQLException e) {
             throw new SystemCommandStatusPersistenceException(format("Failed to find SystemCommandStatus for command id %s", commandId), e);
         }
 
         return systemCommandStatuses;
     }
 
-    public Optional<SystemCommandStatus> findLatestStatus(final UUID commandId) {
+    public Optional<SystemCommandStatus> findLatestStatusById(final UUID commandId) {
         final DataSource systemDataSource = systemJdbcDataSourceProvider.getDataSource();
 
         try (final Connection connection = systemDataSource.getConnection();
-             final PreparedStatement preparedStatement = connection.prepareStatement(FIND_LATEST)) {
+             final PreparedStatement preparedStatement = connection.prepareStatement(FIND_LATEST_BY_ID)) {
 
             preparedStatement.setObject(1, commandId);
 
@@ -135,11 +143,36 @@ public Optional<SystemCommandStatus> findLatestStatus(final UUID commandId) {
                 return empty();
             }
 
-        } catch (SQLException e) {
+        } catch (final SQLException e) {
             throw new SystemCommandStatusPersistenceException("Failed to find latest SystemCommandStatus", e);
         }
     }
 
+    public Optional<SystemCommandStatus> findLatestStatusByType(final SystemCommand systemCommand) {
+        final DataSource systemDataSource = systemJdbcDataSourceProvider.getDataSource();
+
+        try (final Connection connection = systemDataSource.getConnection();
+             final PreparedStatement preparedStatement = connection.prepareStatement(FIND_LATEST_BY_TYPE)) {
+
+            preparedStatement.setString(1, systemCommand.getName());
+
+            try (final ResultSet resultSet = preparedStatement.executeQuery()) {
+
+                if (resultSet.next()) {
+
+                    final SystemCommandStatus systemCommandStatus = toSystemCommandStatus(resultSet);
+
+                    return of(systemCommandStatus);
+                }
+
+                return empty();
+            }
+
+        } catch (final SQLException e) {
+            throw new SystemCommandStatusPersistenceException(format("Failed to find latest SystemCommandStatus for %s", systemCommand.getName()), e);
+        }
+    }
+
     private SystemCommandStatus toSystemCommandStatus(final ResultSet resultSet) throws SQLException {
 
         final UUID commandId = (UUID) resultSet.getObject("command_id");

diff --git a/...and-handling/src/test/java/uk/gov/justice/services/jmx/api/mbean/SystemCommanderTest.java b/...and-handling/src/test/java/uk/gov/justice/services/jmx/api/mbean/SystemCommanderTest.java
@@ -14,7 +14,7 @@
 import static org.mockito.Mockito.when;
 
 import uk.gov.justice.services.jmx.api.CommandNotFoundException;
-import uk.gov.justice.services.jmx.api.UnsupportedSystemCommandException;
+import uk.gov.justice.services.jmx.api.UnrunnableSystemCommandException;
 import uk.gov.justice.services.jmx.api.command.SystemCommand;
 import uk.gov.justice.services.jmx.api.domain.SystemCommandStatus;
 import uk.gov.justice.services.jmx.command.SystemCommandScanner;
@@ -33,7 +33,6 @@
 import org.mockito.runners.MockitoJUnitRunner;
 import org.slf4j.Logger;
 
-
 @RunWith(MockitoJUnitRunner.class)
 public class SystemCommanderTest {
 
@@ -53,12 +52,12 @@ public class SystemCommanderTest {
     private SystemCommander systemCommander;
 
     @Test
-    public void shouldRunTheSystemCommandIfSuppoorted() throws Exception {
+    public void shouldRunTheSystemCommandIfSupported() throws Exception {
 
         final UUID commandId = randomUUID();
         final TestCommand testCommand = new TestCommand();
 
-        when(asynchronousCommandRunner.isSupported(testCommand)).thenReturn(true);
+        when(asynchronousCommandRunner.commandNotSupported(testCommand)).thenReturn(false);
         when(asynchronousCommandRunner.run(testCommand)).thenReturn(commandId);
 
         assertThat(systemCommander.call(testCommand), is(commandId));
@@ -74,16 +73,32 @@ public void shouldFailIfSystemCommandNotSupported() throws Exception {
 
         final TestCommand testCommand = new TestCommand();
 
-        when(asynchronousCommandRunner.isSupported(testCommand)).thenReturn(false);
+        when(asynchronousCommandRunner.commandNotSupported(testCommand)).thenReturn(true);
 
         try {
             systemCommander.call(testCommand);
             fail();
-        } catch (final UnsupportedSystemCommandException expected) {
+        } catch (final UnrunnableSystemCommandException expected) {
             assertThat(expected.getMessage(), is("The system command 'TEST_COMMAND' is not supported on this context."));
         }
     }
 
+    @Test
+    public void shouldFailIfPreviousSystemCommandIsInProgress() throws Exception {
+
+        final TestCommand testCommand = new TestCommand();
+
+        when(asynchronousCommandRunner.commandNotSupported(testCommand)).thenReturn(false);
+        when(systemCommandStateBean.commandInProgress(testCommand)).thenReturn(true);
+
+        try {
+            systemCommander.call(testCommand);
+            fail();
+        } catch (final UnrunnableSystemCommandException expected) {
+            assertThat(expected.getMessage(), is("Cannot run system command 'TEST_COMMAND'. A previous call to that command is still in progress."));
+        }
+    }
+
     @Test
     public void shouldListAllSystemCommands() throws Exception {
 

diff --git a/...dling/src/test/java/uk/gov/justice/services/jmx/runner/AsynchronousCommandRunnerTest.java b/...dling/src/test/java/uk/gov/justice/services/jmx/runner/AsynchronousCommandRunnerTest.java
@@ -90,6 +90,6 @@ public void shouldCheckThatCommandIsSupported() throws Exception {
 
         when(systemCommandRunner.isSupported(systemCommand)).thenReturn(supported);
 
-        assertThat(asynchronousCommandRunner.isSupported(systemCommand), is(supported));
+        assertThat(asynchronousCommandRunner.commandNotSupported(systemCommand), is(false));
     }
 }
diff --git a/...src/test/java/uk/gov/justice/services/jmx/state/observers/SystemCommandStateBeanTest.java b/...src/test/java/uk/gov/justice/services/jmx/state/observers/SystemCommandStateBeanTest.java
@@ -1,13 +1,20 @@
 package uk.gov.justice.services.jmx.state.observers;
 
+import static java.util.Optional.empty;
 import static java.util.Optional.of;
 import static java.util.UUID.randomUUID;
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+import static uk.gov.justice.services.jmx.api.domain.CommandState.COMMAND_COMPLETE;
+import static uk.gov.justice.services.jmx.api.domain.CommandState.COMMAND_FAILED;
+import static uk.gov.justice.services.jmx.api.domain.CommandState.COMMAND_IN_PROGRESS;
+import static uk.gov.justice.services.jmx.api.domain.CommandState.COMMAND_RECEIVED;
 
+import uk.gov.justice.services.jmx.api.command.SystemCommand;
+import uk.gov.justice.services.jmx.api.domain.CommandState;
 import uk.gov.justice.services.jmx.api.domain.SystemCommandStatus;
 import uk.gov.justice.services.jmx.state.persistence.SystemCommandStatusRepository;
 
@@ -46,8 +53,40 @@ public void shouldGetSystemCommandStatusFromTheRepository() throws Exception {
 
         final Optional<SystemCommandStatus> systemCommandStatus = of(mock(SystemCommandStatus.class));
 
-        when(systemCommandStatusRepository.findLatestStatus(commandId)).thenReturn(systemCommandStatus);
+        when(systemCommandStatusRepository.findLatestStatusById(commandId)).thenReturn(systemCommandStatus);
 
         assertThat(systemCommandStateBean.getCommandStatus(commandId), is(systemCommandStatus));
     }
+
+    @Test
+    public void shouldDetermineIfACommandIsInProgress() throws Exception {
+
+        final SystemCommand commandReceivedCommand = mock(SystemCommand.class);
+        final SystemCommand commandInProgressCommand = mock(SystemCommand.class);
+        final SystemCommand commandCompleteCommand = mock(SystemCommand.class);
+        final SystemCommand commandFailedCommand = mock(SystemCommand.class);
+        final SystemCommand notFoundCommand = mock(SystemCommand.class);
+
+        final SystemCommandStatus commandReceivedStatus = mock(SystemCommandStatus.class);
+        final SystemCommandStatus commandInProgressStatus = mock(SystemCommandStatus.class);
+        final SystemCommandStatus commandCompleteStatus = mock(SystemCommandStatus.class);
+        final SystemCommandStatus commandFailedStatus = mock(SystemCommandStatus.class);
+
+        when(systemCommandStatusRepository.findLatestStatusByType(commandReceivedCommand)).thenReturn(of(commandReceivedStatus));
+        when(systemCommandStatusRepository.findLatestStatusByType(commandInProgressCommand)).thenReturn(of(commandInProgressStatus));
+        when(systemCommandStatusRepository.findLatestStatusByType(commandCompleteCommand)).thenReturn(of(commandCompleteStatus));
+        when(systemCommandStatusRepository.findLatestStatusByType(commandFailedCommand)).thenReturn(of(commandFailedStatus));
+        when(systemCommandStatusRepository.findLatestStatusByType(notFoundCommand)).thenReturn(empty());
+
+        when(commandReceivedStatus.getCommandState()).thenReturn(COMMAND_RECEIVED);
+        when(commandInProgressStatus.getCommandState()).thenReturn(COMMAND_IN_PROGRESS);
+        when(commandCompleteStatus.getCommandState()).thenReturn(COMMAND_COMPLETE);
+        when(commandFailedStatus.getCommandState()).thenReturn(COMMAND_FAILED);
+
+        assertThat(systemCommandStateBean.commandInProgress(commandReceivedCommand), is(true));
+        assertThat(systemCommandStateBean.commandInProgress(commandInProgressCommand), is(true));
+        assertThat(systemCommandStateBean.commandInProgress(commandCompleteCommand), is(false));
+        assertThat(systemCommandStateBean.commandInProgress(commandFailedCommand), is(false));
+        assertThat(systemCommandStateBean.commandInProgress(notFoundCommand), is(false));
+    }
 }