From fc12f909188b1fd5a3a7e1d1b78ac3028514e599 Mon Sep 17 00:00:00 2001 From: Matthias Oesterheld Date: Wed, 7 Feb 2024 18:55:59 +0100 Subject: [PATCH 01/14] add migration actions with context --- .../state/actions/MigrationAction.java | 37 ++++++ .../state/actions/MigrationActionContext.java | 68 ++++++++++++ .../state/machine/MigrationStateMachine.java | 4 + .../machine/MigrationStateMachineImpl.java | 19 ++++ .../MigrationStateMachineImplTest.java | 105 ++++++++++++++++++ 5 files changed, 233 insertions(+) create mode 100644 graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationAction.java create mode 100644 graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionContext.java create mode 100644 graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationAction.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationAction.java new file mode 100644 index 000000000000..50bf36b09cb8 --- /dev/null +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationAction.java @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2020 Graylog, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + */ +package org.graylog.plugins.views.storage.migration.state.actions; + +import com.github.oxo42.stateless4j.delegates.Action1; + +import javax.annotation.Nonnull; +import java.util.Objects; + +@FunctionalInterface +@SuppressWarnings("FunctionalInterfaceMethodChanged") // changed for thread safety +public interface MigrationAction extends Action1 { + + @Override + default void doIt(@Nonnull MigrationActionContext context) { + synchronized (Objects.requireNonNull(context)) { + performAction(context); + } + } + + void performAction(MigrationActionContext context); + +} diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionContext.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionContext.java new file mode 100644 index 000000000000..e07b44eabb3f --- /dev/null +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionContext.java @@ -0,0 +1,68 @@ +/* + * Copyright (C) 2020 Graylog, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + */ +package org.graylog.plugins.views.storage.migration.state.actions; + +import org.graylog.plugins.views.storage.migration.state.machine.MigrationState; +import org.graylog.plugins.views.storage.migration.state.machine.MigrationStep; +import org.graylog.plugins.views.storage.migration.state.rest.CurrentStateInformation; + +import java.util.List; +import java.util.Map; + +public class MigrationActionContext { + private final Map args; + private final MigrationState previousState; + private final MigrationStep requestedStep; + + // mutable results + private CurrentStateInformation resultState; + private List errors; + + public MigrationActionContext(Map args, MigrationState previousState, MigrationStep requestedStep) { + this.args = args; + this.previousState = previousState; + this.requestedStep = requestedStep; + } + + public Map getArgs() { + return args; + } + + public MigrationState getPreviousState() { + return previousState; + } + + public MigrationStep getRequestedStep() { + return requestedStep; + } + + public CurrentStateInformation getResultState() { + return resultState; + } + + public List getErrors() { + return errors; + } + + public void setResultState(CurrentStateInformation resultState) { + this.resultState = resultState; + } + + public void setErrors(List errors) { + this.errors = errors; + } +} diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachine.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachine.java index c09d2140e10f..771c80a1c616 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachine.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachine.java @@ -16,10 +16,14 @@ */ package org.graylog.plugins.views.storage.migration.state.machine; +import org.graylog.plugins.views.storage.migration.state.actions.MigrationActionContext; + import java.util.List; import java.util.Map; public interface MigrationStateMachine { + MigrationActionContext triggerWithContext(MigrationStep step, Map args); + MigrationState trigger(MigrationStep step, Map args); MigrationState getState(); diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java index a90f2f7a6c04..3ea9a7d2e2f6 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java @@ -17,7 +17,10 @@ package org.graylog.plugins.views.storage.migration.state.machine; import com.github.oxo42.stateless4j.StateMachine; +import com.github.oxo42.stateless4j.triggers.TriggerWithParameters1; +import org.graylog.plugins.views.storage.migration.state.actions.MigrationActionContext; import org.graylog.plugins.views.storage.migration.state.actions.MigrationActions; +import org.graylog.plugins.views.storage.migration.state.rest.CurrentStateInformation; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -34,6 +37,22 @@ public MigrationStateMachineImpl(StateMachine sta this.migrationActions = migrationActions; } + @Override + public MigrationActionContext triggerWithContext(MigrationStep step, Map args) { + MigrationState currentState = stateMachine.getState(); + MigrationActionContext context = new MigrationActionContext(args, currentState, step); + TriggerWithParameters1 trigger = + new TriggerWithParameters1<>(step, MigrationActionContext.class); + try { + stateMachine.fire(trigger, context); + } catch (Exception e) { + context.setErrors(List.of(e.getMessage())); + } + context.setResultState(new CurrentStateInformation(stateMachine.getState(), stateMachine.getPermittedTriggers())); + return context; + } + + @Override public MigrationState trigger(MigrationStep step, Map args) { try { diff --git a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java new file mode 100644 index 000000000000..3a529b270654 --- /dev/null +++ b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java @@ -0,0 +1,105 @@ +/* + * Copyright (C) 2020 Graylog, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + */ +package org.graylog.plugins.views.storage.migration.state.machine; + +import com.github.oxo42.stateless4j.StateMachine; +import com.github.oxo42.stateless4j.StateMachineConfig; +import com.github.oxo42.stateless4j.triggers.TriggerWithParameters1; +import org.graylog.plugins.views.storage.migration.state.actions.MigrationAction; +import org.graylog.plugins.views.storage.migration.state.actions.MigrationActionContext; +import org.graylog.plugins.views.storage.migration.state.actions.MigrationActions; +import org.junit.jupiter.api.Test; +import org.mockito.Mock; + +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; + +class MigrationStateMachineImplTest { + + MigrationState INITIAL_STATE = MigrationState.NEW; + MigrationState RESULT_STATE = MigrationState.MIGRATION_WELCOME_PAGE; + MigrationStep MIGRATION_STEP = MigrationStep.SELECT_MIGRATION; + + @Mock + MigrationActions migrationActions; //todo: could be removed if context can be used + MigrationStateMachine migrationStateMachine; + + @Test + public void smReturnsResultState() { + StateMachine stateMachine = testStateMachineWithAction((context) -> {}); + migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions); + MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); + assertEquals(RESULT_STATE, context.getResultState().state()); + } + + @Test + public void smPassesArgumentsToAction() { + StateMachine stateMachine = testStateMachineWithAction((context) -> { + assertEquals("v1", context.getArgs().get("arg1")); + assertEquals(2, context.getArgs().get("arg2")); + }); + migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions); + MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of( + "arg1", "v1", "arg2", 2 + )); + } + + @Test + public void smCallsActionOnTransition() { + MigrationAction action = mock(MigrationAction.class); + StateMachine stateMachine = testStateMachineWithAction(action); + migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions); + verifyNoInteractions(action); + migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); + verify(action, times(1)).doIt(any()); + } + + @Test + public void smSetsErrorOnExceptionInAction() { + String errorMessage = "Error 40: Insufficient Coffee."; + StateMachine stateMachine = testStateMachineWithAction((context) -> { + throw new RuntimeException(errorMessage); + }); + migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions); + MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); + assertNotNull(context.getErrors()); + assertFalse(context.getErrors().isEmpty()); + assertEquals(errorMessage, context.getErrors().get(0)); + } + + private StateMachine testStateMachineWithAction(MigrationAction action) { + StateMachineConfig config = new StateMachineConfig<>(); + config.configure(INITIAL_STATE) + .permitDynamic(createDynamicTrigger(MIGRATION_STEP), (context) -> RESULT_STATE, action); + return new StateMachine<>(INITIAL_STATE, config); + } + + private TriggerWithParameters1 createDynamicTrigger(MigrationStep step) { + return new TriggerWithParameters1<>(step, MigrationActionContext.class); + + } + + +} From db5d1b0482bb2e2892866670b36eef8ed60e937a Mon Sep 17 00:00:00 2001 From: Matthias Oesterheld Date: Thu, 8 Feb 2024 10:26:56 +0100 Subject: [PATCH 02/14] add test for unchanged state on exception, avoid npe --- .../state/machine/MigrationStateMachineImpl.java | 4 +++- .../state/machine/MigrationStateMachineImplTest.java | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java index 3ea9a7d2e2f6..f3148b11515d 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java @@ -27,6 +27,7 @@ import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; +import java.util.Objects; public class MigrationStateMachineImpl implements MigrationStateMachine { private final StateMachine stateMachine; @@ -46,7 +47,8 @@ public MigrationActionContext triggerWithContext(MigrationStep step, Map stateMachine = testStateMachineWithAction((context) -> { + throw new RuntimeException(); + }); + migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions); + MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); + assertEquals(context.getPreviousState(), context.getResultState().state()); + } + private StateMachine testStateMachineWithAction(MigrationAction action) { StateMachineConfig config = new StateMachineConfig<>(); config.configure(INITIAL_STATE) From 9bb6caf9d9abedfb9e2a2e1f7a067ea34537f7b7 Mon Sep 17 00:00:00 2001 From: Matthias Oesterheld Date: Thu, 8 Feb 2024 10:48:57 +0100 Subject: [PATCH 03/14] remove unneeded synchronized default method call --- .../state/actions/MigrationAction.java | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationAction.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationAction.java index 50bf36b09cb8..715a258866c4 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationAction.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationAction.java @@ -18,20 +18,9 @@ import com.github.oxo42.stateless4j.delegates.Action1; -import javax.annotation.Nonnull; -import java.util.Objects; - +/** + * Wrapper interface for parameterized State Machine Action1 + */ @FunctionalInterface -@SuppressWarnings("FunctionalInterfaceMethodChanged") // changed for thread safety public interface MigrationAction extends Action1 { - - @Override - default void doIt(@Nonnull MigrationActionContext context) { - synchronized (Objects.requireNonNull(context)) { - performAction(context); - } - } - - void performAction(MigrationActionContext context); - } From d9016273a4a4b12f47fb4c59fd2463820de66b9b Mon Sep 17 00:00:00 2001 From: Matthias Oesterheld Date: Thu, 8 Feb 2024 10:52:00 +0100 Subject: [PATCH 04/14] add test for immutable context fields --- .../state/machine/MigrationStateMachineImplTest.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java index 10131e2e6d21..40db287e1240 100644 --- a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java +++ b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java @@ -46,6 +46,17 @@ class MigrationStateMachineImplTest { MigrationActions migrationActions; //todo: could be removed if context can be used MigrationStateMachine migrationStateMachine; + @Test + public void contextFieldsSet() { + StateMachine stateMachine = testStateMachineWithAction((context) -> {}); + migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions); + Map args = Map.of("k1", "v1"); + MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, args); + assertEquals(INITIAL_STATE, context.getPreviousState()); + assertEquals(MIGRATION_STEP, context.getRequestedStep()); + assertEquals(args, context.getArgs()); + } + @Test public void smReturnsResultState() { StateMachine stateMachine = testStateMachineWithAction((context) -> {}); From 0fc318ef42a556bdfce4e6d854e6318a132c351e Mon Sep 17 00:00:00 2001 From: Matthias Oesterheld Date: Thu, 8 Feb 2024 11:28:29 +0100 Subject: [PATCH 05/14] change error list handling --- .../state/actions/MigrationActionContext.java | 10 ++++++++-- .../state/machine/MigrationStateMachineImpl.java | 2 +- .../state/machine/MigrationStateMachineImplTest.java | 6 +++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionContext.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionContext.java index e07b44eabb3f..f5ac19aaaf9a 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionContext.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionContext.java @@ -20,6 +20,7 @@ import org.graylog.plugins.views.storage.migration.state.machine.MigrationStep; import org.graylog.plugins.views.storage.migration.state.rest.CurrentStateInformation; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -36,6 +37,7 @@ public MigrationActionContext(Map args, MigrationState previousS this.args = args; this.previousState = previousState; this.requestedStep = requestedStep; + this.errors = new ArrayList<>(); } public Map getArgs() { @@ -62,7 +64,11 @@ public void setResultState(CurrentStateInformation resultState) { this.resultState = resultState; } - public void setErrors(List errors) { - this.errors = errors; + public void addError(String error) { + this.errors.add(error); + } + + public boolean hasErrors() { + return !this.errors.isEmpty(); } } diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java index f3148b11515d..4817da8be530 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java @@ -48,7 +48,7 @@ public MigrationActionContext triggerWithContext(MigrationStep step, Map stateMachine = testStateMachineWithAction((context) -> {}); migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions); MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); + assertFalse(context.hasErrors()); assertEquals(RESULT_STATE, context.getResultState().state()); } @@ -95,8 +96,7 @@ public void smSetsErrorOnExceptionInAction() { }); migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions); MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); - assertNotNull(context.getErrors()); - assertFalse(context.getErrors().isEmpty()); + assertTrue(context.hasErrors()); assertEquals(errorMessage, context.getErrors().get(0)); } From 523b6d983f21002f140ef74d68204c745468dc41 Mon Sep 17 00:00:00 2001 From: Matthias Oesterheld Date: Thu, 8 Feb 2024 15:36:41 +0100 Subject: [PATCH 06/14] add test for MigrationStateMachineBuilder move transition actions into `onEntry` fix `permitIf` for `DIRECTORY_COMPATIBILITY_CHECK_PAGE2` --- .../machine/MigrationStateMachineBuilder.java | 15 +- .../MigrationStateMachineBuilderTest.java | 269 ++++++++++++++++++ 2 files changed, 281 insertions(+), 3 deletions(-) create mode 100644 graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilderTest.java diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java index 1b2d47490b2f..9ba20857b44e 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java @@ -19,6 +19,7 @@ import com.github.oxo42.stateless4j.StateMachine; import com.github.oxo42.stateless4j.StateMachineConfig; import com.github.oxo42.stateless4j.delegates.Trace; +import com.google.common.annotations.VisibleForTesting; import org.graylog.plugins.views.storage.migration.state.actions.MigrationActions; import org.graylog.plugins.views.storage.migration.state.persistence.DatanodeMigrationConfiguration; import org.graylog.plugins.views.storage.migration.state.persistence.DatanodeMigrationPersistence; @@ -55,11 +56,12 @@ private static StateMachineConfig configureStates // Major decision - remote reindexing or rolling upgrade(in-place)? config.configure(MigrationState.MIGRATION_SELECTION_PAGE) - .permit(MigrationStep.SELECT_ROLLING_UPGRADE_MIGRATION, MigrationState.ROLLING_UPGRADE_MIGRATION_WELCOME_PAGE, migrationActions::rollingUpgradeSelected) - .permit(MigrationStep.SELECT_REMOTE_REINDEX_MIGRATION, MigrationState.REMOTE_REINDEX_WELCOME_PAGE, migrationActions::reindexUpgradeSelected); + .permit(MigrationStep.SELECT_ROLLING_UPGRADE_MIGRATION, MigrationState.ROLLING_UPGRADE_MIGRATION_WELCOME_PAGE) + .permit(MigrationStep.SELECT_REMOTE_REINDEX_MIGRATION, MigrationState.REMOTE_REINDEX_WELCOME_PAGE); // remote reindexing branch of the migration config.configure(MigrationState.REMOTE_REINDEX_WELCOME_PAGE) + .onEntry(migrationActions::reindexUpgradeSelected) .permit(MigrationStep.DISCOVER_NEW_DATANODES, MigrationState.PROVISION_DATANODE_CERTIFICATES_PAGE, () -> { LOG.info("Remote Reindexing selected"); }); @@ -77,10 +79,11 @@ private static StateMachineConfig configureStates // in place / rolling upgrade branch of the migration config.configure(MigrationState.ROLLING_UPGRADE_MIGRATION_WELCOME_PAGE) + .onEntry(migrationActions::rollingUpgradeSelected) .permit(MigrationStep.INSTALL_DATANODES_ON_EVERY_NODE, MigrationState.DIRECTORY_COMPATIBILITY_CHECK_PAGE2, () -> LOG.info("Showing directory compatibility check page")); config.configure(MigrationState.DIRECTORY_COMPATIBILITY_CHECK_PAGE2) - .permit(MigrationStep.SHOW_PROVISION_ROLLING_UPGRADE_NODES_WITH_CERTIFICATES, MigrationState.PROVISION_ROLLING_UPGRADE_NODES_WITH_CERTIFICATES, migrationActions::directoryCompatibilityCheckOk); + .permitIf(MigrationStep.SHOW_PROVISION_ROLLING_UPGRADE_NODES_WITH_CERTIFICATES, MigrationState.PROVISION_ROLLING_UPGRADE_NODES_WITH_CERTIFICATES, migrationActions::directoryCompatibilityCheckOk); config.configure(MigrationState.PROVISION_ROLLING_UPGRADE_NODES_WITH_CERTIFICATES) .permit(MigrationStep.CALCULATE_JOURNAL_SIZE, MigrationState.JOURNAL_SIZE_DOWNTIME_WARNING); @@ -103,6 +106,12 @@ private static StateMachineConfig configureStates return config; } + @VisibleForTesting + static StateMachine buildWithTestState(MigrationState state, MigrationActions migrationActions) { + final StateMachineConfig config = configureStates(migrationActions); + return new StateMachine<>(state, config); + } + private static StateMachine fromState(MigrationState state, Trace persistence, MigrationActions migrationActions) { final StateMachineConfig config = configureStates(migrationActions); final StateMachine stateMachine = new StateMachine<>(state, config); diff --git a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilderTest.java b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilderTest.java new file mode 100644 index 000000000000..ed076cff7ede --- /dev/null +++ b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilderTest.java @@ -0,0 +1,269 @@ +/* + * Copyright (C) 2020 Graylog, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + */ +package org.graylog.plugins.views.storage.migration.state.machine; + +import com.github.oxo42.stateless4j.StateMachine; +import org.graylog.plugins.views.storage.migration.state.actions.MigrationActions; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +public class MigrationStateMachineBuilderTest { + + @Mock + MigrationActions migrationActions; + + @Test + public void testNewState() { + StateMachine stateMachine = getStateMachine(MigrationState.NEW); + assertThat(stateMachine.getPermittedTriggers()).containsExactly(MigrationStep.SELECT_MIGRATION); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testWelcomePage() { + StateMachine stateMachine = getStateMachine(MigrationState.NEW); + stateMachine.fire(MigrationStep.SELECT_MIGRATION); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.MIGRATION_WELCOME_PAGE); + assertThat(stateMachine.getPermittedTriggers()).containsExactly(MigrationStep.SHOW_DIRECTORY_COMPATIBILITY_CHECK); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testDirectoryCompatibilityCheck() { + StateMachine stateMachine = getStateMachine(MigrationState.MIGRATION_WELCOME_PAGE); + stateMachine.fire(MigrationStep.SHOW_DIRECTORY_COMPATIBILITY_CHECK); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.DIRECTORY_COMPATIBILITY_CHECK_PAGE); + verify(migrationActions).runDirectoryCompatibilityCheck(); + assertThat(stateMachine.getPermittedTriggers()).isEmpty(); + verify(migrationActions, times(2)).directoryCompatibilityCheckOk(); + reset(migrationActions); + when(migrationActions.directoryCompatibilityCheckOk()).thenReturn(true); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_CA_CREATION, MigrationStep.SHOW_RENEWAL_POLICY_CREATION); + verify(migrationActions, times(2)).directoryCompatibilityCheckOk(); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testCaCreationPage() { + when(migrationActions.directoryCompatibilityCheckOk()).thenReturn(true); + StateMachine stateMachine = getStateMachine(MigrationState.DIRECTORY_COMPATIBILITY_CHECK_PAGE); + stateMachine.fire(MigrationStep.SHOW_CA_CREATION); + verify(migrationActions).directoryCompatibilityCheckOk(); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.CA_CREATION_PAGE); + assertThat(stateMachine.getPermittedTriggers()).isEmpty(); + verify(migrationActions, times(1)).caDoesNotExist(); + verify(migrationActions, times(1)).caAndRemovalPolicyExist(); + reset(migrationActions); + when(migrationActions.caDoesNotExist()).thenReturn(true); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_RENEWAL_POLICY_CREATION); + verify(migrationActions, times(1)).caDoesNotExist(); + verify(migrationActions, times(1)).caAndRemovalPolicyExist(); + reset(migrationActions); + when(migrationActions.caDoesNotExist()).thenReturn(false); + when(migrationActions.caAndRemovalPolicyExist()).thenReturn(true); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_MIGRATION_SELECTION); + verify(migrationActions, times(1)).caDoesNotExist(); + verify(migrationActions, times(1)).caAndRemovalPolicyExist(); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testRenewalPolicyCreationPage() { + when(migrationActions.directoryCompatibilityCheckOk()).thenReturn(true); + StateMachine stateMachine = getStateMachine(MigrationState.DIRECTORY_COMPATIBILITY_CHECK_PAGE); + stateMachine.fire(MigrationStep.SHOW_RENEWAL_POLICY_CREATION); + verify(migrationActions).directoryCompatibilityCheckOk(); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.RENEWAL_POLICY_CREATION_PAGE); + assertThat(stateMachine.getPermittedTriggers()).isEmpty(); + verify(migrationActions, times(1)).removalPolicyDoesNotExist(); + verify(migrationActions, times(1)).caAndRemovalPolicyExist(); + reset(migrationActions); + when(migrationActions.removalPolicyDoesNotExist()).thenReturn(true); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_CA_CREATION); + verify(migrationActions, times(1)).removalPolicyDoesNotExist(); + verify(migrationActions, times(1)).caAndRemovalPolicyExist(); + reset(migrationActions); + when(migrationActions.removalPolicyDoesNotExist()).thenReturn(false); + when(migrationActions.caAndRemovalPolicyExist()).thenReturn(true); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_MIGRATION_SELECTION); + verify(migrationActions, times(1)).removalPolicyDoesNotExist(); + verify(migrationActions, times(1)).caAndRemovalPolicyExist(); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testMigrationSelectionPage() { + when(migrationActions.caAndRemovalPolicyExist()).thenReturn(true); + StateMachine stateMachine = getStateMachine(MigrationState.CA_CREATION_PAGE); + stateMachine.fire(MigrationStep.SHOW_MIGRATION_SELECTION); + verify(migrationActions).caAndRemovalPolicyExist(); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.MIGRATION_SELECTION_PAGE); + assertThat(stateMachine.getPermittedTriggers()) + .containsOnly(MigrationStep.SELECT_ROLLING_UPGRADE_MIGRATION, MigrationStep.SELECT_REMOTE_REINDEX_MIGRATION); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testRemoteReindexWelcomePage() { + StateMachine stateMachine = getStateMachine(MigrationState.MIGRATION_SELECTION_PAGE); + stateMachine.fire(MigrationStep.SELECT_REMOTE_REINDEX_MIGRATION); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.REMOTE_REINDEX_WELCOME_PAGE); + verify(migrationActions).reindexUpgradeSelected(); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.DISCOVER_NEW_DATANODES); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testProvisionDatanodeCertificatesPage() { + StateMachine stateMachine = getStateMachine(MigrationState.REMOTE_REINDEX_WELCOME_PAGE); + stateMachine.fire(MigrationStep.DISCOVER_NEW_DATANODES); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.PROVISION_DATANODE_CERTIFICATES_PAGE); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_DATA_MIGRATION_QUESTION); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testExistingDataMigrationQuestionPage() { + StateMachine stateMachine = getStateMachine(MigrationState.PROVISION_DATANODE_CERTIFICATES_PAGE); + stateMachine.fire(MigrationStep.SHOW_DATA_MIGRATION_QUESTION); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.EXISTING_DATA_MIGRATION_QUESTION_PAGE); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_MIGRATE_EXISTING_DATA, MigrationStep.SKIP_EXISTING_DATA_MIGRATION); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testMigrateExistingData() { + StateMachine stateMachine = getStateMachine(MigrationState.EXISTING_DATA_MIGRATION_QUESTION_PAGE); + stateMachine.fire(MigrationStep.SHOW_MIGRATE_EXISTING_DATA); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.MIGRATE_EXISTING_DATA); + verify(migrationActions).reindexOldData(); + assertThat(stateMachine.getPermittedTriggers()).isEmpty(); + verify(migrationActions, times(1)).reindexingFinished(); + reset(migrationActions); + when(migrationActions.reindexingFinished()).thenReturn(true); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_ASK_TO_SHUTDOWN_OLD_CLUSTER); + verify(migrationActions, times(1)).reindexingFinished(); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testRollingUpgradeMigrationWelcomePage() { + StateMachine stateMachine = getStateMachine(MigrationState.MIGRATION_SELECTION_PAGE); + stateMachine.fire(MigrationStep.SELECT_ROLLING_UPGRADE_MIGRATION); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.ROLLING_UPGRADE_MIGRATION_WELCOME_PAGE); + verify(migrationActions).rollingUpgradeSelected(); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.INSTALL_DATANODES_ON_EVERY_NODE); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testDirectoryCompatibilityCheckPage2() { + StateMachine stateMachine = getStateMachine(MigrationState.ROLLING_UPGRADE_MIGRATION_WELCOME_PAGE); + stateMachine.fire(MigrationStep.INSTALL_DATANODES_ON_EVERY_NODE); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.DIRECTORY_COMPATIBILITY_CHECK_PAGE2); + assertThat(stateMachine.getPermittedTriggers()).isEmpty(); + verify(migrationActions, times(1)).directoryCompatibilityCheckOk(); + reset(migrationActions); + when(migrationActions.directoryCompatibilityCheckOk()).thenReturn(true); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_PROVISION_ROLLING_UPGRADE_NODES_WITH_CERTIFICATES); + verify(migrationActions, times(1)).directoryCompatibilityCheckOk(); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testProvisionRollingUpgradeNodesWithCertificates() { + StateMachine stateMachine = getStateMachine(MigrationState.DIRECTORY_COMPATIBILITY_CHECK_PAGE2); + when(migrationActions.directoryCompatibilityCheckOk()).thenReturn(true); + stateMachine.fire(MigrationStep.SHOW_PROVISION_ROLLING_UPGRADE_NODES_WITH_CERTIFICATES); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.PROVISION_ROLLING_UPGRADE_NODES_WITH_CERTIFICATES); + verify(migrationActions, times(1)).directoryCompatibilityCheckOk(); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.CALCULATE_JOURNAL_SIZE); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testJournalSizeDowntimeWarning() { + StateMachine stateMachine = getStateMachine(MigrationState.PROVISION_ROLLING_UPGRADE_NODES_WITH_CERTIFICATES); + stateMachine.fire(MigrationStep.CALCULATE_JOURNAL_SIZE); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.JOURNAL_SIZE_DOWNTIME_WARNING); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_STOP_PROCESSING_PAGE); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testMessageProcessingStopReplaceClusterAndMpRestart() { + StateMachine stateMachine = getStateMachine(MigrationState.JOURNAL_SIZE_DOWNTIME_WARNING); + stateMachine.fire(MigrationStep.SHOW_STOP_PROCESSING_PAGE); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.MESSAGE_PROCESSING_STOP_REPLACE_CLUSTER_AND_MP_RESTART); + verify(migrationActions).stopMessageProcessing(); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_ASK_TO_SHUTDOWN_OLD_CLUSTER); + verifyNoMoreInteractions(migrationActions); + stateMachine.fire(MigrationStep.SHOW_ASK_TO_SHUTDOWN_OLD_CLUSTER); + verify(migrationActions).startMessageProcessing(); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testAskToShutdownOldCluster() { + StateMachine stateMachine = getStateMachine(MigrationState.EXISTING_DATA_MIGRATION_QUESTION_PAGE); + stateMachine.fire(MigrationStep.SKIP_EXISTING_DATA_MIGRATION); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.ASK_TO_SHUTDOWN_OLD_CLUSTER); + assertThat(stateMachine.getPermittedTriggers()).isEmpty(); + verify(migrationActions, times(1)).isOldClusterStopped(); + reset(migrationActions); + when(migrationActions.isOldClusterStopped()).thenReturn(true); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.CONFIRM_OLD_CLUSTER_STOPPED); + verify(migrationActions, times(1)).isOldClusterStopped(); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testAskToShutdownOldClusterFromReindexing() { + StateMachine stateMachine = getStateMachine(MigrationState.MIGRATE_EXISTING_DATA); + when(migrationActions.reindexingFinished()).thenReturn(true); + stateMachine.fire(MigrationStep.SHOW_ASK_TO_SHUTDOWN_OLD_CLUSTER); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.ASK_TO_SHUTDOWN_OLD_CLUSTER); + } + + @Test + public void testManuallyRemoveOldConnectionStringFromConfig() { + StateMachine stateMachine = getStateMachine(MigrationState.ASK_TO_SHUTDOWN_OLD_CLUSTER); + when(migrationActions.isOldClusterStopped()).thenReturn(true); + stateMachine.fire(MigrationStep.CONFIRM_OLD_CLUSTER_STOPPED); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.MANUALLY_REMOVE_OLD_CONNECTION_STRING_FROM_CONFIG); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.CONFIRM_OLD_CONNECTION_STRING_FROM_CONFIG_REMOVED); + stateMachine.fire(MigrationStep.CONFIRM_OLD_CONNECTION_STRING_FROM_CONFIG_REMOVED); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.FINISHED); + } + + @NotNull + private StateMachine getStateMachine(MigrationState initialState) { + return MigrationStateMachineBuilder.buildWithTestState(initialState, migrationActions); + } + +} From 44c88ca40e0f7cc6448b338940795f0fc14ed0d0 Mon Sep 17 00:00:00 2001 From: Matthias Oesterheld Date: Thu, 8 Feb 2024 15:37:45 +0100 Subject: [PATCH 07/14] add error response code --- .../state/machine/MigrationStateMachineImpl.java | 2 +- .../state/rest/CurrentStateInformation.java | 7 ++++++- .../state/rest/MigrationStateResource.java | 15 +++++++++++---- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java index 4817da8be530..c90fcf7ad82a 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java @@ -50,7 +50,7 @@ public MigrationActionContext triggerWithContext(MigrationStep step, Map nextSteps) { +public record CurrentStateInformation(MigrationState state, List nextSteps, List errors) { + + public CurrentStateInformation(MigrationState state, List nextSteps) { + this(state, nextSteps, null); + } + } diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java index 1c611b1d954e..11558bbcf3f2 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java @@ -26,10 +26,12 @@ import jakarta.ws.rs.POST; import jakarta.ws.rs.Path; import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; import org.apache.shiro.authz.annotation.RequiresAuthentication; import org.apache.shiro.authz.annotation.RequiresPermissions; -import org.graylog.plugins.views.storage.migration.state.machine.MigrationState; +import org.graylog.plugins.views.storage.migration.state.actions.MigrationActionContext; import org.graylog.plugins.views.storage.migration.state.machine.MigrationStateMachine; import org.graylog2.audit.jersey.NoAuditEvent; import org.graylog2.shared.security.RestPermissions; @@ -53,9 +55,14 @@ public MigrationStateResource(MigrationStateMachine stateMachine) { @NoAuditEvent("No Audit Event needed") // TODO: do we need audit log here? @RequiresPermissions(RestPermissions.DATANODE_MIGRATION) @ApiOperation(value = "trigger migration step") - public CurrentStateInformation migrate(@ApiParam(name = "request") @NotNull MigrationStepRequest request) { - final MigrationState newState = stateMachine.trigger(request.step(), request.args()); - return new CurrentStateInformation(newState, stateMachine.nextSteps()); + public CurrentStateInformation migrate(@ApiParam(name = "request") @NotNull MigrationStepRequest request, + @Context Response response) { + final MigrationActionContext newState = stateMachine.triggerWithContext(request.step(), request.args()); + return Response.fromResponse(response) + .status(newState.hasErrors() ? Response.Status.INTERNAL_SERVER_ERROR : Response.Status.OK) + .entity(newState.getResultState()) + .build() + .readEntity(CurrentStateInformation.class); } @GET From 29f805d2e92d904a3733017b32443d2ed98df1c6 Mon Sep 17 00:00:00 2001 From: Matthias Oesterheld Date: Mon, 12 Feb 2024 08:44:43 +0100 Subject: [PATCH 08/14] prepare state machine context --- .../state/machine/MigrationStateMachine.java | 4 +- .../machine/MigrationStateMachineContext.java | 59 ++++++++ .../machine/MigrationStateMachineImpl.java | 28 ++-- .../MigrationStateMachineProvider.java | 2 +- .../DatanodeMigrationConfigurationImpl.java | 11 ++ .../DatanodeMigrationPersistence.java | 8 ++ .../state/rest/CurrentStateInformation.java | 2 +- .../state/rest/MigrationStateResource.java | 9 +- .../InMemoryStateMachinePersistence.java | 14 +- .../MigrationStateMachineImplTest.java | 134 ++++++++---------- 10 files changed, 174 insertions(+), 97 deletions(-) create mode 100644 graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineContext.java diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachine.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachine.java index 771c80a1c616..d06490eb624f 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachine.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachine.java @@ -16,13 +16,13 @@ */ package org.graylog.plugins.views.storage.migration.state.machine; -import org.graylog.plugins.views.storage.migration.state.actions.MigrationActionContext; +import org.graylog.plugins.views.storage.migration.state.rest.CurrentStateInformation; import java.util.List; import java.util.Map; public interface MigrationStateMachine { - MigrationActionContext triggerWithContext(MigrationStep step, Map args); + CurrentStateInformation triggerWithContext(MigrationStep step, Map args); MigrationState trigger(MigrationStep step, Map args); MigrationState getState(); diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineContext.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineContext.java new file mode 100644 index 000000000000..73e51c5f3a64 --- /dev/null +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineContext.java @@ -0,0 +1,59 @@ +/* + * Copyright (C) 2020 Graylog, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + */ +package org.graylog.plugins.views.storage.migration.state.machine; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.auto.value.AutoValue; + +import java.util.HashMap; +import java.util.Map; + +@AutoValue +public abstract class MigrationStateMachineContext { + + @JsonProperty("action_arguments") + public abstract Map actionArguments(); + + public abstract Builder toBuilder(); + + public MigrationStateMachineContext withArguments(MigrationStep step, Object args) { + Map currentArgs = actionArguments(); + currentArgs.put(step, args); + return toBuilder().actionArguments(currentArgs).build(); + } + + @JsonCreator + public static MigrationStateMachineContext create() { + return builder() + .actionArguments(new HashMap<>()) + .build(); + } + + public static Builder builder() { + return new AutoValue_MigrationStateMachineContext.Builder(); + } + + + @AutoValue.Builder + public abstract static class Builder { + @JsonProperty("action_arguments") + public abstract Builder actionArguments(Map actionArguments); + + public abstract MigrationStateMachineContext build(); + } +} diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java index c90fcf7ad82a..bf991b7b4822 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java @@ -17,9 +17,8 @@ package org.graylog.plugins.views.storage.migration.state.machine; import com.github.oxo42.stateless4j.StateMachine; -import com.github.oxo42.stateless4j.triggers.TriggerWithParameters1; -import org.graylog.plugins.views.storage.migration.state.actions.MigrationActionContext; import org.graylog.plugins.views.storage.migration.state.actions.MigrationActions; +import org.graylog.plugins.views.storage.migration.state.persistence.DatanodeMigrationPersistence; import org.graylog.plugins.views.storage.migration.state.rest.CurrentStateInformation; import java.io.ByteArrayOutputStream; @@ -32,26 +31,29 @@ public class MigrationStateMachineImpl implements MigrationStateMachine { private final StateMachine stateMachine; private final MigrationActions migrationActions; + private final DatanodeMigrationPersistence persistenceService; + private MigrationStateMachineContext context; - public MigrationStateMachineImpl(StateMachine stateMachine, MigrationActions migrationActions) { + public MigrationStateMachineImpl(StateMachine stateMachine, MigrationActions migrationActions, DatanodeMigrationPersistence persistenceService) { this.stateMachine = stateMachine; this.migrationActions = migrationActions; + this.persistenceService = persistenceService; + this.context = persistenceService.getStateMachineContext().orElse(MigrationStateMachineContext.create()); } @Override - public MigrationActionContext triggerWithContext(MigrationStep step, Map args) { - MigrationState currentState = stateMachine.getState(); - MigrationActionContext context = new MigrationActionContext(args, currentState, step); - TriggerWithParameters1 trigger = - new TriggerWithParameters1<>(step, MigrationActionContext.class); + public CurrentStateInformation triggerWithContext(MigrationStep step, Map args) { + if (Objects.nonNull(args) && !args.isEmpty()) { + this.context = context.withArguments(step, args); + persistenceService.saveStateMachineContext(context); + } + String errorMessage = null; try { - stateMachine.fire(trigger, context); + stateMachine.fire(step); } catch (Exception e) { - String message = (Objects.nonNull(e.getMessage())) ? e.getMessage() : e.getClass().getName(); - context.addError(message); + errorMessage = Objects.nonNull(e.getMessage()) ? e.getMessage() : e.toString(); } - context.setResultState(new CurrentStateInformation(stateMachine.getState(), stateMachine.getPermittedTriggers(), context.getErrors())); - return context; + return new CurrentStateInformation(getState(), nextSteps(), errorMessage); } diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineProvider.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineProvider.java index d228c8547b73..e104f65495eb 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineProvider.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineProvider.java @@ -38,6 +38,6 @@ public MigrationStateMachineProvider(DatanodeMigrationPersistence persistenceSer @Override public MigrationStateMachine get() { final StateMachine stateMachine = MigrationStateMachineBuilder.buildFromPersistedState(persistenceService, migrationActions); - return new MigrationStateMachineImpl(stateMachine, migrationActions); + return new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); } } diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/persistence/DatanodeMigrationConfigurationImpl.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/persistence/DatanodeMigrationConfigurationImpl.java index 44555127979e..14761e54fb74 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/persistence/DatanodeMigrationConfigurationImpl.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/persistence/DatanodeMigrationConfigurationImpl.java @@ -18,6 +18,7 @@ import jakarta.inject.Inject; import jakarta.inject.Singleton; +import org.graylog.plugins.views.storage.migration.state.machine.MigrationStateMachineContext; import org.graylog2.plugin.cluster.ClusterConfigService; import java.util.Optional; @@ -41,4 +42,14 @@ public Optional getConfiguration() { public void saveConfiguration(DatanodeMigrationConfiguration configuration) { clusterConfigService.write(configuration); } + + @Override + public Optional getStateMachineContext() { + return Optional.ofNullable(clusterConfigService.get(MigrationStateMachineContext.class)); + } + + @Override + public void saveStateMachineContext(MigrationStateMachineContext context) { + clusterConfigService.write(context); + } } diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/persistence/DatanodeMigrationPersistence.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/persistence/DatanodeMigrationPersistence.java index c6afeb10f812..5d1a82fa2889 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/persistence/DatanodeMigrationPersistence.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/persistence/DatanodeMigrationPersistence.java @@ -16,9 +16,17 @@ */ package org.graylog.plugins.views.storage.migration.state.persistence; +import org.graylog.plugins.views.storage.migration.state.machine.MigrationStateMachineContext; + import java.util.Optional; public interface DatanodeMigrationPersistence { + Optional getConfiguration(); + void saveConfiguration(DatanodeMigrationConfiguration configuration); + + Optional getStateMachineContext(); + + void saveStateMachineContext(MigrationStateMachineContext context); } diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/CurrentStateInformation.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/CurrentStateInformation.java index c1ad029787e4..1ff36d05e387 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/CurrentStateInformation.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/CurrentStateInformation.java @@ -21,7 +21,7 @@ import java.util.List; -public record CurrentStateInformation(MigrationState state, List nextSteps, List errors) { +public record CurrentStateInformation(MigrationState state, List nextSteps, String errorMessage) { public CurrentStateInformation(MigrationState state, List nextSteps) { this(state, nextSteps, null); diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java index 11558bbcf3f2..f4a189b25373 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java @@ -31,7 +31,6 @@ import jakarta.ws.rs.core.Response; import org.apache.shiro.authz.annotation.RequiresAuthentication; import org.apache.shiro.authz.annotation.RequiresPermissions; -import org.graylog.plugins.views.storage.migration.state.actions.MigrationActionContext; import org.graylog.plugins.views.storage.migration.state.machine.MigrationStateMachine; import org.graylog2.audit.jersey.NoAuditEvent; import org.graylog2.shared.security.RestPermissions; @@ -57,12 +56,8 @@ public MigrationStateResource(MigrationStateMachine stateMachine) { @ApiOperation(value = "trigger migration step") public CurrentStateInformation migrate(@ApiParam(name = "request") @NotNull MigrationStepRequest request, @Context Response response) { - final MigrationActionContext newState = stateMachine.triggerWithContext(request.step(), request.args()); - return Response.fromResponse(response) - .status(newState.hasErrors() ? Response.Status.INTERNAL_SERVER_ERROR : Response.Status.OK) - .entity(newState.getResultState()) - .build() - .readEntity(CurrentStateInformation.class); + final CurrentStateInformation newState = stateMachine.triggerWithContext(request.step(), request.args()); + return newState; } @GET diff --git a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/InMemoryStateMachinePersistence.java b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/InMemoryStateMachinePersistence.java index 832787fcb2aa..a04651fa0b32 100644 --- a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/InMemoryStateMachinePersistence.java +++ b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/InMemoryStateMachinePersistence.java @@ -16,9 +16,8 @@ */ package org.graylog.plugins.views.storage.migration.state; -import com.github.oxo42.stateless4j.delegates.Trace; import org.graylog.plugins.views.storage.migration.state.machine.MigrationState; -import org.graylog.plugins.views.storage.migration.state.machine.MigrationStep; +import org.graylog.plugins.views.storage.migration.state.machine.MigrationStateMachineContext; import org.graylog.plugins.views.storage.migration.state.persistence.DatanodeMigrationConfiguration; import org.graylog.plugins.views.storage.migration.state.persistence.DatanodeMigrationPersistence; @@ -27,6 +26,7 @@ public class InMemoryStateMachinePersistence implements DatanodeMigrationPersistence { private MigrationState currentState = MigrationState.NEW; + private MigrationStateMachineContext context = MigrationStateMachineContext.create(); @Override public Optional getConfiguration() { @@ -37,4 +37,14 @@ public Optional getConfiguration() { public void saveConfiguration(DatanodeMigrationConfiguration configuration) { this.currentState = configuration.currentState(); } + + @Override + public Optional getStateMachineContext() { + return Optional.of(context); + } + + @Override + public void saveStateMachineContext(MigrationStateMachineContext context) { + this.context = context; + } } diff --git a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java index 7c3b116a5396..13ad873954cc 100644 --- a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java +++ b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java @@ -18,24 +18,17 @@ import com.github.oxo42.stateless4j.StateMachine; import com.github.oxo42.stateless4j.StateMachineConfig; +import com.github.oxo42.stateless4j.delegates.Action; import com.github.oxo42.stateless4j.triggers.TriggerWithParameters1; -import org.graylog.plugins.views.storage.migration.state.actions.MigrationAction; +import org.graylog.plugins.views.storage.migration.state.InMemoryStateMachinePersistence; import org.graylog.plugins.views.storage.migration.state.actions.MigrationActionContext; import org.graylog.plugins.views.storage.migration.state.actions.MigrationActions; +import org.graylog.plugins.views.storage.migration.state.rest.CurrentStateInformation; import org.junit.jupiter.api.Test; import org.mockito.Mock; import java.util.Map; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; - class MigrationStateMachineImplTest { MigrationState INITIAL_STATE = MigrationState.NEW; @@ -44,76 +37,75 @@ class MigrationStateMachineImplTest { @Mock MigrationActions migrationActions; //todo: could be removed if context can be used + @Mock + InMemoryStateMachinePersistence persistenceService = new InMemoryStateMachinePersistence(); MigrationStateMachine migrationStateMachine; @Test public void contextFieldsSet() { - StateMachine stateMachine = testStateMachineWithAction((context) -> {}); - migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions); + StateMachine stateMachine = testStateMachineWithAction(() -> {}); + migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); Map args = Map.of("k1", "v1"); - MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, args); - assertEquals(INITIAL_STATE, context.getPreviousState()); - assertEquals(MIGRATION_STEP, context.getRequestedStep()); - assertEquals(args, context.getArgs()); - } - - @Test - public void smReturnsResultState() { - StateMachine stateMachine = testStateMachineWithAction((context) -> {}); - migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions); - MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); - assertFalse(context.hasErrors()); - assertEquals(RESULT_STATE, context.getResultState().state()); - } - - @Test - public void smPassesArgumentsToAction() { - StateMachine stateMachine = testStateMachineWithAction((context) -> { - assertEquals("v1", context.getArgs().get("arg1")); - assertEquals(2, context.getArgs().get("arg2")); - }); - migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions); - MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of( - "arg1", "v1", "arg2", 2 - )); - } - - @Test - public void smCallsActionOnTransition() { - MigrationAction action = mock(MigrationAction.class); - StateMachine stateMachine = testStateMachineWithAction(action); - migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions); - verifyNoInteractions(action); - migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); - verify(action, times(1)).doIt(any()); - } - - @Test - public void smSetsErrorOnExceptionInAction() { - String errorMessage = "Error 40: Insufficient Coffee."; - StateMachine stateMachine = testStateMachineWithAction((context) -> { - throw new RuntimeException(errorMessage); - }); - migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions); - MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); - assertTrue(context.hasErrors()); - assertEquals(errorMessage, context.getErrors().get(0)); - } - - @Test - public void smStateUnchangedOnExceptionInAction() { - StateMachine stateMachine = testStateMachineWithAction((context) -> { - throw new RuntimeException(); - }); - migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions); - MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); - assertEquals(context.getPreviousState(), context.getResultState().state()); + CurrentStateInformation context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, args); } - private StateMachine testStateMachineWithAction(MigrationAction action) { +// @Test +// public void smReturnsResultState() { +// StateMachine stateMachine = testStateMachineWithAction((context) -> {}); +// migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); +// MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); +// assertFalse(context.hasErrors()); +// assertEquals(RESULT_STATE, context.getResultState().state()); +// } +// +// @Test +// public void smPassesArgumentsToAction() { +// StateMachine stateMachine = testStateMachineWithAction((context) -> { +// assertEquals("v1", context.getArgs().get("arg1")); +// assertEquals(2, context.getArgs().get("arg2")); +// }); +// migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); +// MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of( +// "arg1", "v1", "arg2", 2 +// )); +// } +// +// @Test +// public void smCallsActionOnTransition() { +// MigrationAction action = mock(MigrationAction.class); +// StateMachine stateMachine = testStateMachineWithAction(action); +// migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); +// verifyNoInteractions(action); +// migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); +// verify(action, times(1)).doIt(any()); +// } +// +// @Test +// public void smSetsErrorOnExceptionInAction() { +// String errorMessage = "Error 40: Insufficient Coffee."; +// StateMachine stateMachine = testStateMachineWithAction((context) -> { +// throw new RuntimeException(errorMessage); +// }); +// migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); +// MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); +// assertTrue(context.hasErrors()); +// assertEquals(errorMessage, context.getErrors().get(0)); +// } +// +// @Test +// public void smStateUnchangedOnExceptionInAction() { +// StateMachine stateMachine = testStateMachineWithAction((context) -> { +// throw new RuntimeException(); +// }); +// migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); +// MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); +// assertEquals(context.getPreviousState(), context.getResultState().state()); +// } + + private StateMachine testStateMachineWithAction(Action action) { StateMachineConfig config = new StateMachineConfig<>(); config.configure(INITIAL_STATE) - .permitDynamic(createDynamicTrigger(MIGRATION_STEP), (context) -> RESULT_STATE, action); + .permit(MIGRATION_STEP, RESULT_STATE, action); return new StateMachine<>(INITIAL_STATE, config); } From 21d8876833bd8ed32fcd127dd09e4f641ccb01b5 Mon Sep 17 00:00:00 2001 From: Matthias Oesterheld Date: Mon, 12 Feb 2024 08:45:07 +0100 Subject: [PATCH 09/14] add onEntry Test --- .../state/machine/MigrationStateMachineBuilder.java | 6 ++++++ .../state/machine/MigrationStateMachineBuilderTest.java | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java index 9ba20857b44e..7fe787fcd9bc 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java @@ -33,6 +33,12 @@ public class MigrationStateMachineBuilder { @NotNull private static StateMachineConfig configureStates(MigrationActions migrationActions) { + + // All actions should be performed on transaction to make sure that there is no state change on error. + // For async tasks, the task should be triggered in the transition with a following intermediary step which + // has a guard to continue only on task completion. + + StateMachineConfig config = new StateMachineConfig<>(); config.configure(MigrationState.NEW) diff --git a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilderTest.java b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilderTest.java index ed076cff7ede..874a72ade5a2 100644 --- a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilderTest.java +++ b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilderTest.java @@ -261,6 +261,14 @@ public void testManuallyRemoveOldConnectionStringFromConfig() { assertThat(stateMachine.getState()).isEqualTo(MigrationState.FINISHED); } + @Test + public void testOnEntryAction() { + StateMachine stateMachine = getStateMachine(MigrationState.MIGRATE_EXISTING_DATA); + stateMachine.fireInitialTransition(); + verify(migrationActions).reindexOldData(); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.MIGRATE_EXISTING_DATA); + } + @NotNull private StateMachine getStateMachine(MigrationState initialState) { return MigrationStateMachineBuilder.buildWithTestState(initialState, migrationActions); From 86dc2b0eb62acd3696da0850580b811446c6104a Mon Sep 17 00:00:00 2001 From: Matthias Oesterheld Date: Mon, 12 Feb 2024 16:35:02 +0100 Subject: [PATCH 10/14] remove WithArgs to use MigrationStateMachineContext --- .../state/actions/MigrationAction.java | 26 --- .../state/actions/MigrationActionContext.java | 74 ------- .../state/actions/MigrationActions.java | 8 +- .../state/actions/MigrationActionsImpl.java | 15 ++ .../migration/state/actions/WithArgs.java | 45 ----- .../state/machine/MigrationStateMachine.java | 6 +- .../machine/MigrationStateMachineContext.java | 61 +++--- .../machine/MigrationStateMachineImpl.java | 22 +-- .../state/rest/CurrentStateInformation.java | 5 + .../state/rest/MigrationStateResource.java | 9 +- .../InMemoryStateMachinePersistence.java | 2 +- .../state/MigrationStateMachineTest.java | 4 +- .../machine/MigrationActionsAdapter.java | 10 + .../MigrationStateMachineImplTest.java | 181 +++++++++++------- 14 files changed, 208 insertions(+), 260 deletions(-) delete mode 100644 graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationAction.java delete mode 100644 graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionContext.java delete mode 100644 graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/WithArgs.java diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationAction.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationAction.java deleted file mode 100644 index 715a258866c4..000000000000 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationAction.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright (C) 2020 Graylog, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the Server Side Public License, version 1, - * as published by MongoDB, Inc. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * Server Side Public License for more details. - * - * You should have received a copy of the Server Side Public License - * along with this program. If not, see - * . - */ -package org.graylog.plugins.views.storage.migration.state.actions; - -import com.github.oxo42.stateless4j.delegates.Action1; - -/** - * Wrapper interface for parameterized State Machine Action1 - */ -@FunctionalInterface -public interface MigrationAction extends Action1 { -} diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionContext.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionContext.java deleted file mode 100644 index f5ac19aaaf9a..000000000000 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionContext.java +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright (C) 2020 Graylog, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the Server Side Public License, version 1, - * as published by MongoDB, Inc. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * Server Side Public License for more details. - * - * You should have received a copy of the Server Side Public License - * along with this program. If not, see - * . - */ -package org.graylog.plugins.views.storage.migration.state.actions; - -import org.graylog.plugins.views.storage.migration.state.machine.MigrationState; -import org.graylog.plugins.views.storage.migration.state.machine.MigrationStep; -import org.graylog.plugins.views.storage.migration.state.rest.CurrentStateInformation; - -import java.util.ArrayList; -import java.util.List; -import java.util.Map; - -public class MigrationActionContext { - private final Map args; - private final MigrationState previousState; - private final MigrationStep requestedStep; - - // mutable results - private CurrentStateInformation resultState; - private List errors; - - public MigrationActionContext(Map args, MigrationState previousState, MigrationStep requestedStep) { - this.args = args; - this.previousState = previousState; - this.requestedStep = requestedStep; - this.errors = new ArrayList<>(); - } - - public Map getArgs() { - return args; - } - - public MigrationState getPreviousState() { - return previousState; - } - - public MigrationStep getRequestedStep() { - return requestedStep; - } - - public CurrentStateInformation getResultState() { - return resultState; - } - - public List getErrors() { - return errors; - } - - public void setResultState(CurrentStateInformation resultState) { - this.resultState = resultState; - } - - public void addError(String error) { - this.errors.add(error); - } - - public boolean hasErrors() { - return !this.errors.isEmpty(); - } -} diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActions.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActions.java index 70dfd47996d2..1fe0568c524b 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActions.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActions.java @@ -16,12 +16,12 @@ */ package org.graylog.plugins.views.storage.migration.state.actions; -import java.util.Map; +import org.graylog.plugins.views.storage.migration.state.machine.MigrationStateMachineContext; /** * Set of callbacks used during the migration in different states. */ -public interface MigrationActions extends WithArgs { +public interface MigrationActions { boolean runDirectoryCompatibilityCheck(); boolean isOldClusterStopped(); @@ -43,4 +43,8 @@ public interface MigrationActions extends WithArgs { boolean removalPolicyDoesNotExist(); boolean caAndRemovalPolicyExist(); void resetMigration(); + + void setStateMachineContext(MigrationStateMachineContext context); + + MigrationStateMachineContext getStateMachineContext(); } diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionsImpl.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionsImpl.java index 4fe3e76827a2..b4204884b995 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionsImpl.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/MigrationActionsImpl.java @@ -17,16 +17,20 @@ package org.graylog.plugins.views.storage.migration.state.actions; import jakarta.inject.Inject; +import jakarta.inject.Singleton; +import org.graylog.plugins.views.storage.migration.state.machine.MigrationStateMachineContext; import org.graylog.plugins.views.storage.migration.state.persistence.DatanodeMigrationConfiguration; import org.graylog.security.certutil.CaService; import org.graylog.security.certutil.ca.exceptions.KeyStoreStorageException; import org.graylog2.plugin.certificates.RenewalPolicy; import org.graylog2.plugin.cluster.ClusterConfigService; +@Singleton public class MigrationActionsImpl implements MigrationActions { private final ClusterConfigService clusterConfigService; private final CaService caService; + private MigrationStateMachineContext stateMachineContext; @Inject public MigrationActionsImpl(final ClusterConfigService clusterConfigService, @@ -108,4 +112,15 @@ public boolean removalPolicyDoesNotExist() { public boolean caAndRemovalPolicyExist() { return !caDoesNotExist() && !removalPolicyDoesNotExist(); } + + @Override + public void setStateMachineContext(MigrationStateMachineContext context) { + this.stateMachineContext = context; + } + + @Override + public MigrationStateMachineContext getStateMachineContext() { + return stateMachineContext; + } + } diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/WithArgs.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/WithArgs.java deleted file mode 100644 index a4653a0e4ff8..000000000000 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/actions/WithArgs.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright (C) 2020 Graylog, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the Server Side Public License, version 1, - * as published by MongoDB, Inc. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * Server Side Public License for more details. - * - * You should have received a copy of the Server Side Public License - * along with this program. If not, see - * . - */ -package org.graylog.plugins.views.storage.migration.state.actions; - -import java.util.Map; - -/** - * This allows handling request parameters inside {@link MigrationActions}. The original state machine - * doesn't support action parameters and typed triggers are clumsy to use. In the same time there is no - * simple way to extend the state machine to support parameters in transition actions. - * - * With this interface, we support them inside {@link MigrationActions} where we actually need them. - */ -public interface WithArgs { - ThreadLocal> requestArguments = new ThreadLocal<>(); - - /** - * @return request arguments provided by the frontend to the state machine transition action - */ - default Map args() { - return requestArguments.get(); - } - - default void setArgs(Map args) { - requestArguments.set(args); - } - - default void clearArgs() { - requestArguments.remove(); - } -} diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachine.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachine.java index d06490eb624f..3f97fef22b67 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachine.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachine.java @@ -22,12 +22,14 @@ import java.util.Map; public interface MigrationStateMachine { - CurrentStateInformation triggerWithContext(MigrationStep step, Map args); - MigrationState trigger(MigrationStep step, Map args); + CurrentStateInformation trigger(MigrationStep step, Map args); + MigrationState getState(); List nextSteps(); + MigrationStateMachineContext getContext(); + String serialize(); } diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineContext.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineContext.java index 73e51c5f3a64..018ac258ef7d 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineContext.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineContext.java @@ -16,44 +16,57 @@ */ package org.graylog.plugins.views.storage.migration.state.machine; -import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonAutoDetect; import com.fasterxml.jackson.annotation.JsonProperty; -import com.google.auto.value.AutoValue; import java.util.HashMap; import java.util.Map; +import java.util.Objects; -@AutoValue -public abstract class MigrationStateMachineContext { +@JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.PROTECTED_AND_PUBLIC) +public class MigrationStateMachineContext { - @JsonProperty("action_arguments") - public abstract Map actionArguments(); + @JsonProperty + protected MigrationStep currentStep; + @JsonProperty + protected Map> actionArguments; + @JsonProperty + protected Map extendedState; - public abstract Builder toBuilder(); - - public MigrationStateMachineContext withArguments(MigrationStep step, Object args) { - Map currentArgs = actionArguments(); - currentArgs.put(step, args); - return toBuilder().actionArguments(currentArgs).build(); + public MigrationStateMachineContext() { + this.actionArguments = new HashMap<>(); + this.extendedState = new HashMap<>(); } - @JsonCreator - public static MigrationStateMachineContext create() { - return builder() - .actionArguments(new HashMap<>()) - .build(); + public void setCurrentStep(MigrationStep currentStep) { + this.currentStep = currentStep; } - public static Builder builder() { - return new AutoValue_MigrationStateMachineContext.Builder(); + public T getActionArgument(String name, Class type) { + Map args = this.actionArguments.get(currentStep); + if (Objects.isNull(args)) { + throw new IllegalArgumentException("Missing arguments for step " + currentStep); + } + if (!args.containsKey(name)) { + throw new IllegalArgumentException("Missing argument " + name + " for step " + currentStep); + } + Object arg = args.get(name); + if (!type.isInstance(arg)) { + throw new IllegalArgumentException("Argument " + name + " must be of type " + type); + } + return (T) arg; } + public void addActionArguments(MigrationStep step, Map args) { + this.actionArguments.put(step, args); + } - @AutoValue.Builder - public abstract static class Builder { - @JsonProperty("action_arguments") - public abstract Builder actionArguments(Map actionArguments); + public void addExtendedState(String key, Object value) { + this.extendedState.put(key, value); + } - public abstract MigrationStateMachineContext build(); + public Object getExtendedState(String key) { + return this.extendedState.get(key); } + } diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java index bf991b7b4822..4154df044ef9 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImpl.java @@ -38,34 +38,30 @@ public MigrationStateMachineImpl(StateMachine sta this.stateMachine = stateMachine; this.migrationActions = migrationActions; this.persistenceService = persistenceService; - this.context = persistenceService.getStateMachineContext().orElse(MigrationStateMachineContext.create()); + this.context = persistenceService.getStateMachineContext().orElse(new MigrationStateMachineContext()); } @Override - public CurrentStateInformation triggerWithContext(MigrationStep step, Map args) { + public CurrentStateInformation trigger(MigrationStep step, Map args) { + context.setCurrentStep(step); if (Objects.nonNull(args) && !args.isEmpty()) { - this.context = context.withArguments(step, args); - persistenceService.saveStateMachineContext(context); + context.addActionArguments(step, args); } + migrationActions.setStateMachineContext(context); String errorMessage = null; try { stateMachine.fire(step); } catch (Exception e) { errorMessage = Objects.nonNull(e.getMessage()) ? e.getMessage() : e.toString(); } + context = migrationActions.getStateMachineContext(); + persistenceService.saveStateMachineContext(context); return new CurrentStateInformation(getState(), nextSteps(), errorMessage); } - @Override - public MigrationState trigger(MigrationStep step, Map args) { - try { - migrationActions.setArgs(args); - stateMachine.fire(step); - } finally { - migrationActions.clearArgs(); - } - return stateMachine.getState(); + public MigrationStateMachineContext getContext() { + return context; } @Override diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/CurrentStateInformation.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/CurrentStateInformation.java index 1ff36d05e387..e001c7b16722 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/CurrentStateInformation.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/CurrentStateInformation.java @@ -16,6 +16,7 @@ */ package org.graylog.plugins.views.storage.migration.state.rest; +import org.apache.commons.lang3.StringUtils; import org.graylog.plugins.views.storage.migration.state.machine.MigrationState; import org.graylog.plugins.views.storage.migration.state.machine.MigrationStep; @@ -27,4 +28,8 @@ public CurrentStateInformation(MigrationState state, List nextSte this(state, nextSteps, null); } + public boolean hasErrors() { + return StringUtils.isNotBlank(errorMessage); + } + } diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java index f4a189b25373..2a54cb91e114 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java @@ -29,6 +29,7 @@ import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; +import org.apache.commons.lang3.StringUtils; import org.apache.shiro.authz.annotation.RequiresAuthentication; import org.apache.shiro.authz.annotation.RequiresPermissions; import org.graylog.plugins.views.storage.migration.state.machine.MigrationStateMachine; @@ -56,8 +57,12 @@ public MigrationStateResource(MigrationStateMachine stateMachine) { @ApiOperation(value = "trigger migration step") public CurrentStateInformation migrate(@ApiParam(name = "request") @NotNull MigrationStepRequest request, @Context Response response) { - final CurrentStateInformation newState = stateMachine.triggerWithContext(request.step(), request.args()); - return newState; + final CurrentStateInformation newState = stateMachine.trigger(request.step(), request.args()); + return Response.fromResponse(response) + .status(StringUtils.isEmpty(newState.errorMessage()) ? Response.Status.OK : Response.Status.INTERNAL_SERVER_ERROR) + .entity(newState) + .build() + .readEntity(CurrentStateInformation.class); } @GET diff --git a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/InMemoryStateMachinePersistence.java b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/InMemoryStateMachinePersistence.java index a04651fa0b32..152017d87416 100644 --- a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/InMemoryStateMachinePersistence.java +++ b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/InMemoryStateMachinePersistence.java @@ -26,7 +26,7 @@ public class InMemoryStateMachinePersistence implements DatanodeMigrationPersistence { private MigrationState currentState = MigrationState.NEW; - private MigrationStateMachineContext context = MigrationStateMachineContext.create(); + private MigrationStateMachineContext context = new MigrationStateMachineContext(); @Override public Optional getConfiguration() { diff --git a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/MigrationStateMachineTest.java b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/MigrationStateMachineTest.java index 664069af2833..d094c74bdc59 100644 --- a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/MigrationStateMachineTest.java +++ b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/MigrationStateMachineTest.java @@ -17,14 +17,12 @@ package org.graylog.plugins.views.storage.migration.state; import org.assertj.core.api.Assertions; -import org.graylog.plugins.views.storage.migration.state.actions.MigrationActions; import org.graylog.plugins.views.storage.migration.state.machine.MigrationActionsAdapter; import org.graylog.plugins.views.storage.migration.state.machine.MigrationState; import org.graylog.plugins.views.storage.migration.state.machine.MigrationStateMachine; import org.graylog.plugins.views.storage.migration.state.machine.MigrationStateMachineProvider; import org.graylog.plugins.views.storage.migration.state.machine.MigrationStep; import org.junit.jupiter.api.Test; -import org.mockito.Mockito; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,7 +49,7 @@ void testPersistence() { @Override public boolean runDirectoryCompatibilityCheck() { - capturedArgs.set(args()); + //capturedArgs.set(args()); return true; } diff --git a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationActionsAdapter.java b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationActionsAdapter.java index 06aa86973a88..ed0bf33dbe85 100644 --- a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationActionsAdapter.java +++ b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationActionsAdapter.java @@ -24,6 +24,16 @@ public void resetMigration() { } + @Override + public void setStateMachineContext(MigrationStateMachineContext context) { + + } + + @Override + public MigrationStateMachineContext getStateMachineContext() { + return null; + } + @Override public boolean runDirectoryCompatibilityCheck() { return false; diff --git a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java index 13ad873954cc..707d6541881c 100644 --- a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java +++ b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java @@ -18,16 +18,18 @@ import com.github.oxo42.stateless4j.StateMachine; import com.github.oxo42.stateless4j.StateMachineConfig; -import com.github.oxo42.stateless4j.delegates.Action; -import com.github.oxo42.stateless4j.triggers.TriggerWithParameters1; import org.graylog.plugins.views.storage.migration.state.InMemoryStateMachinePersistence; -import org.graylog.plugins.views.storage.migration.state.actions.MigrationActionContext; import org.graylog.plugins.views.storage.migration.state.actions.MigrationActions; +import org.graylog.plugins.views.storage.migration.state.actions.MigrationActionsImpl; import org.graylog.plugins.views.storage.migration.state.rest.CurrentStateInformation; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.Mock; import java.util.Map; +import java.util.function.Consumer; + +import static org.assertj.core.api.Assertions.assertThat; + class MigrationStateMachineImplTest { @@ -35,82 +37,125 @@ class MigrationStateMachineImplTest { MigrationState RESULT_STATE = MigrationState.MIGRATION_WELCOME_PAGE; MigrationStep MIGRATION_STEP = MigrationStep.SELECT_MIGRATION; - @Mock - MigrationActions migrationActions; //todo: could be removed if context can be used - @Mock + MigrationActions migrationActions; InMemoryStateMachinePersistence persistenceService = new InMemoryStateMachinePersistence(); MigrationStateMachine migrationStateMachine; + @BeforeEach + public void setUp() { + migrationActions = new TestMigrationActions(); + } + + @Test + public void smReturnsResultState() { + StateMachine stateMachine = testStateMachineWithAction((context) -> {}); + migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); + CurrentStateInformation context = migrationStateMachine.trigger(MIGRATION_STEP, Map.of()); + assertThat(context.hasErrors()).isFalse(); + assertThat(context.state()).isEqualTo(RESULT_STATE); + } + + @Test + public void smPassesArgumentsToAction() { + StateMachine stateMachine = testStateMachineWithAction((context) -> { + assertThat(context.getActionArgument("arg1", String.class)).isEqualTo("v1"); + assertThat(context.getActionArgument("arg2", Integer.class)).isEqualTo(2); + }); + migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); + CurrentStateInformation context = migrationStateMachine.trigger(MIGRATION_STEP, Map.of( + "arg1", "v1", "arg2", 2 + )); + assertThat(context.hasErrors()).isFalse(); + } + @Test - public void contextFieldsSet() { - StateMachine stateMachine = testStateMachineWithAction(() -> {}); + public void smActionCanSetExtendedState() { + StateMachine stateMachine = testStateMachineWithAction((context) -> { + int i = context.getActionArgument("k1", Integer.class); + context.addExtendedState("r1", ++i); + }); migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); - Map args = Map.of("k1", "v1"); - CurrentStateInformation context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, args); + CurrentStateInformation context = migrationStateMachine.trigger(MIGRATION_STEP, Map.of("k1", 1)); + assertThat(context.hasErrors()).isFalse(); + assertThat(migrationStateMachine.getContext().getExtendedState("r1")).isEqualTo(2); } -// @Test -// public void smReturnsResultState() { -// StateMachine stateMachine = testStateMachineWithAction((context) -> {}); -// migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); -// MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); -// assertFalse(context.hasErrors()); -// assertEquals(RESULT_STATE, context.getResultState().state()); -// } -// -// @Test -// public void smPassesArgumentsToAction() { -// StateMachine stateMachine = testStateMachineWithAction((context) -> { -// assertEquals("v1", context.getArgs().get("arg1")); -// assertEquals(2, context.getArgs().get("arg2")); -// }); -// migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); -// MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of( -// "arg1", "v1", "arg2", 2 -// )); -// } -// -// @Test -// public void smCallsActionOnTransition() { -// MigrationAction action = mock(MigrationAction.class); -// StateMachine stateMachine = testStateMachineWithAction(action); -// migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); -// verifyNoInteractions(action); -// migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); -// verify(action, times(1)).doIt(any()); -// } -// -// @Test -// public void smSetsErrorOnExceptionInAction() { -// String errorMessage = "Error 40: Insufficient Coffee."; -// StateMachine stateMachine = testStateMachineWithAction((context) -> { -// throw new RuntimeException(errorMessage); -// }); -// migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); -// MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); -// assertTrue(context.hasErrors()); -// assertEquals(errorMessage, context.getErrors().get(0)); -// } -// -// @Test -// public void smStateUnchangedOnExceptionInAction() { -// StateMachine stateMachine = testStateMachineWithAction((context) -> { -// throw new RuntimeException(); -// }); -// migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); -// MigrationActionContext context = migrationStateMachine.triggerWithContext(MIGRATION_STEP, Map.of()); -// assertEquals(context.getPreviousState(), context.getResultState().state()); -// } - - private StateMachine testStateMachineWithAction(Action action) { + @Test + public void smThrowsErrorOnNoArguments() { + StateMachine stateMachine = testStateMachineWithAction((context) -> { + context.getActionArgument("away", String.class); + }); + migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); + CurrentStateInformation context = migrationStateMachine.trigger(MIGRATION_STEP, null); + assertThat(context.hasErrors()).isTrue(); + assertThat(context.errorMessage()).isEqualTo("Missing arguments for step SELECT_MIGRATION"); + } + + @Test + public void smThrowsErrorOnMissingArgument() { + StateMachine stateMachine = testStateMachineWithAction((context) -> { + context.getActionArgument("away", String.class); + }); + migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); + CurrentStateInformation context = migrationStateMachine.trigger(MIGRATION_STEP, Map.of("k1", "v1")); + assertThat(context.hasErrors()).isTrue(); + assertThat(context.errorMessage()).isEqualTo("Missing argument away for step SELECT_MIGRATION"); + } + + @Test + public void smThrowsErrorOnWrongArgumentType() { + StateMachine stateMachine = testStateMachineWithAction((context) -> { + context.getActionArgument("k1", Integer.class); + }); + migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); + CurrentStateInformation context = migrationStateMachine.trigger(MIGRATION_STEP, Map.of("k1", "v1")); + assertThat(context.hasErrors()).isTrue(); + assertThat(context.errorMessage()).isEqualTo("Missing argument away for step SELECT_MIGRATION"); + } + + @Test + public void smSetsErrorOnExceptionInAction() { + String errorMessage = "Error 40: Insufficient Coffee."; + StateMachine stateMachine = testStateMachineWithAction((context) -> { + throw new RuntimeException(errorMessage); + }); + migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); + CurrentStateInformation context = migrationStateMachine.trigger(MIGRATION_STEP, Map.of()); + assertThat(context.hasErrors()).isTrue(); + assertThat(context.errorMessage()).isEqualTo(errorMessage); + } + + @Test + public void smStateUnchangedOnExceptionInAction() { + StateMachine stateMachine = testStateMachineWithAction((context) -> { + throw new RuntimeException(); + }); + migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); + CurrentStateInformation context = migrationStateMachine.trigger(MIGRATION_STEP, Map.of()); + assertThat(context.hasErrors()).isTrue(); + assertThat(context.state()).isEqualTo(INITIAL_STATE); + } + + private StateMachine testStateMachineWithAction(Consumer action) { StateMachineConfig config = new StateMachineConfig<>(); config.configure(INITIAL_STATE) - .permit(MIGRATION_STEP, RESULT_STATE, action); + .permit(MIGRATION_STEP, RESULT_STATE, () -> { + TestMigrationActions testMigrationActions = (TestMigrationActions) migrationActions; + testMigrationActions.runTestFunction(action); + }); return new StateMachine<>(INITIAL_STATE, config); } - private TriggerWithParameters1 createDynamicTrigger(MigrationStep step) { - return new TriggerWithParameters1<>(step, MigrationActionContext.class); + + private static class TestMigrationActions extends MigrationActionsImpl { + + public TestMigrationActions() { + super(null, null); + } + + public void runTestFunction(Consumer testFunction) { + testFunction.accept(getStateMachineContext()); + } } From 3100cfcc45aa1d42e4d5d78c1122326cebdb1045 Mon Sep 17 00:00:00 2001 From: Matthias Oesterheld Date: Mon, 12 Feb 2024 17:29:38 +0100 Subject: [PATCH 11/14] adapt to sm changes --- .../MigrationStateMachineBuilderTest.java | 56 +++++++++---------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilderTest.java b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilderTest.java index 874a72ade5a2..a560109259cf 100644 --- a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilderTest.java +++ b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilderTest.java @@ -46,10 +46,9 @@ public void testNewState() { @Test public void testWelcomePage() { - StateMachine stateMachine = getStateMachine(MigrationState.NEW); - stateMachine.fire(MigrationStep.SELECT_MIGRATION); + StateMachine stateMachine = getStateMachine(MigrationState.MIGRATION_WELCOME_PAGE); assertThat(stateMachine.getState()).isEqualTo(MigrationState.MIGRATION_WELCOME_PAGE); - assertThat(stateMachine.getPermittedTriggers()).containsExactly(MigrationStep.SHOW_DIRECTORY_COMPATIBILITY_CHECK); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_DIRECTORY_COMPATIBILITY_CHECK, MigrationStep.SKIP_DIRECTORY_COMPATIBILITY_CHECK); verifyNoMoreInteractions(migrationActions); } @@ -60,11 +59,11 @@ public void testDirectoryCompatibilityCheck() { assertThat(stateMachine.getState()).isEqualTo(MigrationState.DIRECTORY_COMPATIBILITY_CHECK_PAGE); verify(migrationActions).runDirectoryCompatibilityCheck(); assertThat(stateMachine.getPermittedTriggers()).isEmpty(); - verify(migrationActions, times(2)).directoryCompatibilityCheckOk(); + verify(migrationActions, times(1)).directoryCompatibilityCheckOk(); reset(migrationActions); when(migrationActions.directoryCompatibilityCheckOk()).thenReturn(true); - assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_CA_CREATION, MigrationStep.SHOW_RENEWAL_POLICY_CREATION); - verify(migrationActions, times(2)).directoryCompatibilityCheckOk(); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_CA_CREATION); + verify(migrationActions, times(1)).directoryCompatibilityCheckOk(); verifyNoMoreInteractions(migrationActions); } @@ -75,18 +74,18 @@ public void testCaCreationPage() { stateMachine.fire(MigrationStep.SHOW_CA_CREATION); verify(migrationActions).directoryCompatibilityCheckOk(); assertThat(stateMachine.getState()).isEqualTo(MigrationState.CA_CREATION_PAGE); + when(migrationActions.caDoesNotExist()).thenReturn(true); assertThat(stateMachine.getPermittedTriggers()).isEmpty(); verify(migrationActions, times(1)).caDoesNotExist(); verify(migrationActions, times(1)).caAndRemovalPolicyExist(); reset(migrationActions); - when(migrationActions.caDoesNotExist()).thenReturn(true); assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_RENEWAL_POLICY_CREATION); verify(migrationActions, times(1)).caDoesNotExist(); verify(migrationActions, times(1)).caAndRemovalPolicyExist(); reset(migrationActions); when(migrationActions.caDoesNotExist()).thenReturn(false); when(migrationActions.caAndRemovalPolicyExist()).thenReturn(true); - assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_MIGRATION_SELECTION); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_RENEWAL_POLICY_CREATION, MigrationStep.SHOW_MIGRATION_SELECTION); verify(migrationActions, times(1)).caDoesNotExist(); verify(migrationActions, times(1)).caAndRemovalPolicyExist(); verifyNoMoreInteractions(migrationActions); @@ -94,24 +93,16 @@ public void testCaCreationPage() { @Test public void testRenewalPolicyCreationPage() { - when(migrationActions.directoryCompatibilityCheckOk()).thenReturn(true); - StateMachine stateMachine = getStateMachine(MigrationState.DIRECTORY_COMPATIBILITY_CHECK_PAGE); + when(migrationActions.caDoesNotExist()).thenReturn(false); + StateMachine stateMachine = getStateMachine(MigrationState.CA_CREATION_PAGE); stateMachine.fire(MigrationStep.SHOW_RENEWAL_POLICY_CREATION); - verify(migrationActions).directoryCompatibilityCheckOk(); + verify(migrationActions).caDoesNotExist(); assertThat(stateMachine.getState()).isEqualTo(MigrationState.RENEWAL_POLICY_CREATION_PAGE); assertThat(stateMachine.getPermittedTriggers()).isEmpty(); - verify(migrationActions, times(1)).removalPolicyDoesNotExist(); - verify(migrationActions, times(1)).caAndRemovalPolicyExist(); - reset(migrationActions); - when(migrationActions.removalPolicyDoesNotExist()).thenReturn(true); - assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_CA_CREATION); - verify(migrationActions, times(1)).removalPolicyDoesNotExist(); verify(migrationActions, times(1)).caAndRemovalPolicyExist(); reset(migrationActions); - when(migrationActions.removalPolicyDoesNotExist()).thenReturn(false); when(migrationActions.caAndRemovalPolicyExist()).thenReturn(true); assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_MIGRATION_SELECTION); - verify(migrationActions, times(1)).removalPolicyDoesNotExist(); verify(migrationActions, times(1)).caAndRemovalPolicyExist(); verifyNoMoreInteractions(migrationActions); } @@ -143,14 +134,28 @@ public void testProvisionDatanodeCertificatesPage() { StateMachine stateMachine = getStateMachine(MigrationState.REMOTE_REINDEX_WELCOME_PAGE); stateMachine.fire(MigrationStep.DISCOVER_NEW_DATANODES); assertThat(stateMachine.getState()).isEqualTo(MigrationState.PROVISION_DATANODE_CERTIFICATES_PAGE); + assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.PROVISION_DATANODE_CERTIFICATES); + verifyNoMoreInteractions(migrationActions); + } + + @Test + public void testProvisionDatanodeCertificatesRunning() { + StateMachine stateMachine = getStateMachine(MigrationState.PROVISION_DATANODE_CERTIFICATES_PAGE); + stateMachine.fire(MigrationStep.PROVISION_DATANODE_CERTIFICATES); + verify(migrationActions, times(1)).provisionDataNodes(); + assertThat(stateMachine.getState()).isEqualTo(MigrationState.PROVISION_DATANODE_CERTIFICATES_RUNNING); + assertThat(stateMachine.getPermittedTriggers()).isEmpty(); + verify(migrationActions, times(1)).provisioningFinished(); + reset(migrationActions); + when(migrationActions.provisioningFinished()).thenReturn(true); assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_DATA_MIGRATION_QUESTION); + verify(migrationActions, times(1)).provisioningFinished(); verifyNoMoreInteractions(migrationActions); } @Test public void testExistingDataMigrationQuestionPage() { - StateMachine stateMachine = getStateMachine(MigrationState.PROVISION_DATANODE_CERTIFICATES_PAGE); - stateMachine.fire(MigrationStep.SHOW_DATA_MIGRATION_QUESTION); + StateMachine stateMachine = getStateMachine(MigrationState.EXISTING_DATA_MIGRATION_QUESTION_PAGE); assertThat(stateMachine.getState()).isEqualTo(MigrationState.EXISTING_DATA_MIGRATION_QUESTION_PAGE); assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_MIGRATE_EXISTING_DATA, MigrationStep.SKIP_EXISTING_DATA_MIGRATION); verifyNoMoreInteractions(migrationActions); @@ -212,6 +217,7 @@ public void testJournalSizeDowntimeWarning() { stateMachine.fire(MigrationStep.CALCULATE_JOURNAL_SIZE); assertThat(stateMachine.getState()).isEqualTo(MigrationState.JOURNAL_SIZE_DOWNTIME_WARNING); assertThat(stateMachine.getPermittedTriggers()).containsOnly(MigrationStep.SHOW_STOP_PROCESSING_PAGE); + verify(migrationActions, times(1)).provisionDataNodes(); verifyNoMoreInteractions(migrationActions); } @@ -261,14 +267,6 @@ public void testManuallyRemoveOldConnectionStringFromConfig() { assertThat(stateMachine.getState()).isEqualTo(MigrationState.FINISHED); } - @Test - public void testOnEntryAction() { - StateMachine stateMachine = getStateMachine(MigrationState.MIGRATE_EXISTING_DATA); - stateMachine.fireInitialTransition(); - verify(migrationActions).reindexOldData(); - assertThat(stateMachine.getState()).isEqualTo(MigrationState.MIGRATE_EXISTING_DATA); - } - @NotNull private StateMachine getStateMachine(MigrationState initialState) { return MigrationStateMachineBuilder.buildWithTestState(initialState, migrationActions); From 22dfb552b8c311256a26595321be14f4eada72af Mon Sep 17 00:00:00 2001 From: Matthias Oesterheld Date: Mon, 12 Feb 2024 17:30:00 +0100 Subject: [PATCH 12/14] merge interface changes --- .../state/machine/MigrationStateMachineBuilder.java | 4 ++-- .../migration/state/MigrationStateMachineTest.java | 4 +++- .../state/machine/MigrationActionsAdapter.java | 11 +++++++++-- .../state/machine/MigrationStateMachineImplTest.java | 4 ++-- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java index aa07908a0a79..8d7f87629fec 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java @@ -34,11 +34,10 @@ public class MigrationStateMachineBuilder { @NotNull private static StateMachineConfig configureStates(MigrationActions migrationActions) { - // All actions should be performed on transaction to make sure that there is no state change on error. + // All actions which can fail should be performed on transaction to make sure that there is no state change on error. // For async tasks, the task should be triggered in the transition with a following intermediary step which // has a guard to continue only on task completion. - StateMachineConfig config = new StateMachineConfig<>(); config.configure(MigrationState.NEW) @@ -123,6 +122,7 @@ static StateMachine buildWithTestState(MigrationS private static StateMachine fromState(MigrationState state, Trace persistence, MigrationActions migrationActions) { final StateMachineConfig config = configureStates(migrationActions); final StateMachine stateMachine = new StateMachine<>(state, config); + stateMachine.fireInitialTransition(); // asserts that onEntry will be performed when loading persisted state stateMachine.setTrace(persistence); return stateMachine; } diff --git a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/MigrationStateMachineTest.java b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/MigrationStateMachineTest.java index d094c74bdc59..479239bf9a91 100644 --- a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/MigrationStateMachineTest.java +++ b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/MigrationStateMachineTest.java @@ -49,7 +49,9 @@ void testPersistence() { @Override public boolean runDirectoryCompatibilityCheck() { - //capturedArgs.set(args()); + String key = "foo"; + String value = getStateMachineContext().getActionArgument(key, String.class); + capturedArgs.set(Map.of(key, value)); return true; } diff --git a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationActionsAdapter.java b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationActionsAdapter.java index c6cebce049d0..b028e6ec8eb5 100644 --- a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationActionsAdapter.java +++ b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationActionsAdapter.java @@ -19,6 +19,13 @@ import org.graylog.plugins.views.storage.migration.state.actions.MigrationActions; public class MigrationActionsAdapter implements MigrationActions { + + MigrationStateMachineContext context; + + public MigrationActionsAdapter() { + this.context = new MigrationStateMachineContext(); + } + @Override public void resetMigration() { @@ -26,12 +33,12 @@ public void resetMigration() { @Override public void setStateMachineContext(MigrationStateMachineContext context) { - + this.context = context; } @Override public MigrationStateMachineContext getStateMachineContext() { - return null; + return context; } @Override diff --git a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java index 707d6541881c..b551baf92777 100644 --- a/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java +++ b/graylog2-server/src/test/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineImplTest.java @@ -110,7 +110,7 @@ public void smThrowsErrorOnWrongArgumentType() { migrationStateMachine = new MigrationStateMachineImpl(stateMachine, migrationActions, persistenceService); CurrentStateInformation context = migrationStateMachine.trigger(MIGRATION_STEP, Map.of("k1", "v1")); assertThat(context.hasErrors()).isTrue(); - assertThat(context.errorMessage()).isEqualTo("Missing argument away for step SELECT_MIGRATION"); + assertThat(context.errorMessage()).isEqualTo("Argument k1 must be of type class java.lang.Integer"); } @Test @@ -150,7 +150,7 @@ private StateMachine testStateMachineWithAction(C private static class TestMigrationActions extends MigrationActionsImpl { public TestMigrationActions() { - super(null, null); + super(null, null, null, null); } public void runTestFunction(Consumer testFunction) { From 7ab9a810952b3710cb249357298e7ae8a286a832 Mon Sep 17 00:00:00 2001 From: Matthias Oesterheld Date: Mon, 12 Feb 2024 17:47:42 +0100 Subject: [PATCH 13/14] typo --- .../migration/state/machine/MigrationStateMachineBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java index 8d7f87629fec..b03fa6f357d1 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/machine/MigrationStateMachineBuilder.java @@ -34,7 +34,7 @@ public class MigrationStateMachineBuilder { @NotNull private static StateMachineConfig configureStates(MigrationActions migrationActions) { - // All actions which can fail should be performed on transaction to make sure that there is no state change on error. + // All actions which can fail should be performed on transition to make sure that there is no state change on error. // For async tasks, the task should be triggered in the transition with a following intermediary step which // has a guard to continue only on task completion. From 2b135e922545d4e711084f441ea3ef0a536f29eb Mon Sep 17 00:00:00 2001 From: Matthias Oesterheld Date: Mon, 12 Feb 2024 17:48:48 +0100 Subject: [PATCH 14/14] use `hasErrors` method --- .../storage/migration/state/rest/MigrationStateResource.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java index 2a54cb91e114..9dd806178e7e 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/storage/migration/state/rest/MigrationStateResource.java @@ -29,7 +29,6 @@ import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; -import org.apache.commons.lang3.StringUtils; import org.apache.shiro.authz.annotation.RequiresAuthentication; import org.apache.shiro.authz.annotation.RequiresPermissions; import org.graylog.plugins.views.storage.migration.state.machine.MigrationStateMachine; @@ -59,7 +58,7 @@ public CurrentStateInformation migrate(@ApiParam(name = "request") @NotNull Migr @Context Response response) { final CurrentStateInformation newState = stateMachine.trigger(request.step(), request.args()); return Response.fromResponse(response) - .status(StringUtils.isEmpty(newState.errorMessage()) ? Response.Status.OK : Response.Status.INTERNAL_SERVER_ERROR) + .status(newState.hasErrors() ? Response.Status.INTERNAL_SERVER_ERROR : Response.Status.OK) .entity(newState) .build() .readEntity(CurrentStateInformation.class);