diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java index fdad2fb8aada..3d26e6c8f3d0 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java @@ -29,6 +29,7 @@ public enum FeatureFlagEnum { release_git_autocommit_feature_enabled, release_git_autocommit_eligibility_enabled, release_dynamodb_connection_time_to_live_enabled, + release_reactive_actions_enabled, // Add EE flags below this line, to avoid conflicts. } diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/RunBehaviourEnum.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/RunBehaviourEnum.java index f14f65791be0..a65550978266 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/RunBehaviourEnum.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/RunBehaviourEnum.java @@ -5,5 +5,6 @@ */ public enum RunBehaviourEnum { MANUAL, // Action will only run when manually triggered - ON_PAGE_LOAD // Action will run when the page loads + ON_PAGE_LOAD, // Action will run when the page loads + AUTOMATIC // Action will run automatically (When any of its dependencies change) if the feature flag is enabled } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java index 692056474797..3d8143e2be93 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java @@ -2,6 +2,7 @@ import com.appsmith.external.dtos.DslExecutableDTO; import com.appsmith.external.dtos.LayoutExecutableUpdateDTO; +import com.appsmith.external.enums.FeatureFlagEnum; import com.appsmith.external.helpers.MustacheHelper; import com.appsmith.external.models.ActionDTO; import com.appsmith.external.models.CreatorContextType; @@ -15,9 +16,11 @@ import com.appsmith.server.domains.NewPage; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; +import com.appsmith.server.helpers.CollectionUtils; import com.appsmith.server.helpers.ObservationHelperImpl; import com.appsmith.server.onload.executables.ExecutableOnLoadService; import com.appsmith.server.services.AstService; +import com.appsmith.server.services.FeatureFlagService; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import io.micrometer.observation.ObservationRegistry; @@ -90,6 +93,7 @@ public class OnLoadExecutablesUtilCEImpl implements OnLoadExecutablesUtilCE { private final Set APPSMITH_GLOBAL_VARIABLES = Set.of(); private final ObservationRegistry observationRegistry; private final ObservationHelperImpl observationHelper; + private final FeatureFlagService featureFlagService; /** * This function computes the sequenced on page load executables. @@ -320,103 +324,117 @@ public Mono updateExecutablesRunBehaviour( return existingOnLoadExecutablesMono .zipWith(creatorContextExecutablesFlux.collectList()) - .flatMap(tuple -> { - List existingOnLoadExecutables = tuple.getT1(); - List creatorContextExecutables = tuple.getT2(); + .flatMap(tuple -> featureFlagService + .check(FeatureFlagEnum.release_reactive_actions_enabled) + .flatMap(isReactiveActionsEnabled -> { + List existingOnLoadExecutables = tuple.getT1(); + List creatorContextExecutables = tuple.getT2(); + + // There are no actions in this page. No need to proceed further since no actions would get + // updated + if (CollectionUtils.isNullOrEmpty(creatorContextExecutables)) { + return Mono.just(FALSE); + } - // There are no actions in this page. No need to proceed further since no actions would get updated - if (creatorContextExecutables.isEmpty()) { - return Mono.just(FALSE); - } + // No actions require an update if no actions have been found as page load actions as well + // as + // existing on load page actions are empty + if (CollectionUtils.isNullOrEmpty(existingOnLoadExecutables) + && (onLoadExecutables == null + || CollectionUtils.isNullOrEmpty(onLoadExecutables))) { + return Mono.just(FALSE); + } - // No actions require an update if no actions have been found as page load actions as well as - // existing on load page actions are empty - if (existingOnLoadExecutables.isEmpty() - && (onLoadExecutables == null || onLoadExecutables.isEmpty())) { - return Mono.just(FALSE); - } + // Extract names of existing page load actions and new page load actions for quick lookup. + Set existingOnLoadExecutableNames = existingOnLoadExecutables.stream() + .map(Executable::getUserExecutableName) + .collect(Collectors.toSet()); - // Extract names of existing page load actions and new page load actions for quick lookup. - Set existingOnLoadExecutableNames = existingOnLoadExecutables.stream() - .map(Executable::getUserExecutableName) - .collect(Collectors.toSet()); - - Set newOnLoadExecutableNames = onLoadExecutables.stream() - .map(Executable::getUserExecutableName) - .collect(Collectors.toSet()); - - // Calculate the actions which would need to be updated from execute on load TRUE to FALSE. - Set turnedOffExecutableNames = new HashSet<>(); - turnedOffExecutableNames.addAll(existingOnLoadExecutableNames); - turnedOffExecutableNames.removeAll(newOnLoadExecutableNames); - - // Calculate the actions which would need to be updated from execute on load FALSE to TRUE - Set turnedOnExecutableNames = new HashSet<>(); - turnedOnExecutableNames.addAll(newOnLoadExecutableNames); - turnedOnExecutableNames.removeAll(existingOnLoadExecutableNames); - - for (Executable executable : creatorContextExecutables) { - - String executableName = executable.getUserExecutableName(); - // If a user has ever set execute on load, this field can not be changed automatically. It has - // to be explicitly changed by the user again. Add the executable to update only if this - // condition is false. - if (FALSE.equals(executable.getUserSetOnLoad())) { - - // If this executable is no longer an onload executable, turn the execute on load to false - if (turnedOffExecutableNames.contains(executableName)) { - executable.setRunBehaviour(RunBehaviourEnum.MANUAL); - toUpdateExecutables.add(executable); - } + Set newOnLoadExecutableNames = onLoadExecutables.stream() + .map(Executable::getUserExecutableName) + .collect(Collectors.toSet()); - // If this executable is newly found to be on load, turn execute on load to true - if (turnedOnExecutableNames.contains(executableName)) { - executable.setRunBehaviour(RunBehaviourEnum.ON_PAGE_LOAD); - toUpdateExecutables.add(executable); - } + // Calculate the actions which would need to be updated from execute on load TRUE to FALSE. + Set turnedOffExecutableNames = new HashSet<>(); + turnedOffExecutableNames.addAll(existingOnLoadExecutableNames); + turnedOffExecutableNames.removeAll(newOnLoadExecutableNames); + + // Calculate the actions which would need to be updated from execute on load FALSE to TRUE + Set turnedOnExecutableNames = new HashSet<>(); + turnedOnExecutableNames.addAll(newOnLoadExecutableNames); + turnedOnExecutableNames.removeAll(existingOnLoadExecutableNames); + + for (Executable executable : creatorContextExecutables) { + + String executableName = executable.getUserExecutableName(); + // If a user has ever set execute on load, this field can not be changed automatically. + // It has + // to be explicitly changed by the user again. Add the executable to update only if this + // condition is false. + if (FALSE.equals(executable.getUserSetOnLoad())) { + + // If this executable is no longer an onload executable, turn the execute on load to + // false + if (turnedOffExecutableNames.contains(executableName)) { + executable.setRunBehaviour(RunBehaviourEnum.MANUAL); + toUpdateExecutables.add(executable); + } - } else { - // Remove the executable name from either of the lists (if present) because this executable - // should not be updated - turnedOnExecutableNames.remove(executableName); - turnedOffExecutableNames.remove(executableName); - } - } + // If this executable is newly found to be on load, turn execute on load to true + if (turnedOnExecutableNames.contains(executableName)) { + // Use the prefetched feature flag value + if (isReactiveActionsEnabled) { + executable.setRunBehaviour(RunBehaviourEnum.AUTOMATIC); + } else { + executable.setRunBehaviour(RunBehaviourEnum.ON_PAGE_LOAD); + } + toUpdateExecutables.add(executable); + } - // Add newly turned on page actions to report back to the caller - executableUpdatesRef.addAll( - addExecutableUpdatesForExecutableNames(creatorContextExecutables, turnedOnExecutableNames)); + } else { + // Remove the executable name from either of the lists (if present) because this + // executable + // should not be updated + turnedOnExecutableNames.remove(executableName); + turnedOffExecutableNames.remove(executableName); + } + } - // Add newly turned off page actions to report back to the caller - executableUpdatesRef.addAll(addExecutableUpdatesForExecutableNames( - creatorContextExecutables, turnedOffExecutableNames)); + // Add newly turned on page actions to report back to the caller + executableUpdatesRef.addAll(addExecutableUpdatesForExecutableNames( + creatorContextExecutables, turnedOnExecutableNames)); - for (Executable executable : creatorContextExecutables) { - String executableName = executable.getUserExecutableName(); - if (Boolean.FALSE.equals(executable.isOnLoadMessageAllowed())) { - turnedOffExecutableNames.remove(executableName); - turnedOnExecutableNames.remove(executableName); - } - } + // Add newly turned off page actions to report back to the caller + executableUpdatesRef.addAll(addExecutableUpdatesForExecutableNames( + creatorContextExecutables, turnedOffExecutableNames)); - // Now add messagesRef that would eventually be displayed to the developer user informing them - // about the action setting change. - if (!turnedOffExecutableNames.isEmpty()) { - messagesRef.add( - turnedOffExecutableNames.toString() + " will no longer be executed on page load"); - } + for (Executable executable : creatorContextExecutables) { + String executableName = executable.getUserExecutableName(); + if (Boolean.FALSE.equals(executable.isOnLoadMessageAllowed())) { + turnedOffExecutableNames.remove(executableName); + turnedOnExecutableNames.remove(executableName); + } + } - if (!turnedOnExecutableNames.isEmpty()) { - messagesRef.add( - turnedOnExecutableNames.toString() + " will be executed automatically on page load"); - } + // Now add messagesRef that would eventually be displayed to the developer user informing + // them + // about the action setting change. + if (!turnedOffExecutableNames.isEmpty()) { + messagesRef.add(turnedOffExecutableNames.toString() + + " will no longer be executed on page load"); + } - // Finally update the actions which require an update - return Flux.fromIterable(toUpdateExecutables) - .flatMap(executable -> - this.updateUnpublishedExecutable(executable.getId(), executable, creatorType)) - .then(Mono.just(TRUE)); - }); + if (!turnedOnExecutableNames.isEmpty()) { + messagesRef.add(turnedOnExecutableNames.toString() + + " will be executed automatically on page load"); + } + + // Finally update the actions which require an update + return Flux.fromIterable(toUpdateExecutables) + .flatMap(executable -> this.updateUnpublishedExecutable( + executable.getId(), executable, creatorType)) + .then(Mono.just(TRUE)); + })); } @Override diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilImpl.java index acb26e326cbe..a6bd2f180652 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilImpl.java @@ -4,6 +4,7 @@ import com.appsmith.server.helpers.ObservationHelperImpl; import com.appsmith.server.onload.executables.ExecutableOnLoadService; import com.appsmith.server.services.AstService; +import com.appsmith.server.services.FeatureFlagService; import com.fasterxml.jackson.databind.ObjectMapper; import io.micrometer.observation.ObservationRegistry; import lombok.extern.slf4j.Slf4j; @@ -18,7 +19,14 @@ public OnLoadExecutablesUtilImpl( ObjectMapper objectMapper, ExecutableOnLoadService pageExecutableOnLoadService, ObservationRegistry observationRegistry, - ObservationHelperImpl observationHelper) { - super(astService, objectMapper, pageExecutableOnLoadService, observationRegistry, observationHelper); + ObservationHelperImpl observationHelper, + FeatureFlagService featureFlagService) { + super( + astService, + objectMapper, + pageExecutableOnLoadService, + observationRegistry, + observationHelper, + featureFlagService); } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImplTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImplTest.java new file mode 100644 index 000000000000..8ab3205d3b7a --- /dev/null +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImplTest.java @@ -0,0 +1,256 @@ +package com.appsmith.server.onload.internal; + +import com.appsmith.external.dtos.LayoutExecutableUpdateDTO; +import com.appsmith.external.enums.FeatureFlagEnum; +import com.appsmith.external.models.ActionDTO; +import com.appsmith.external.models.CreatorContextType; +import com.appsmith.external.models.Executable; +import com.appsmith.external.models.RunBehaviourEnum; +import com.appsmith.server.helpers.ObservationHelperImpl; +import com.appsmith.server.onload.executables.ExecutableOnLoadService; +import com.appsmith.server.services.AstService; +import com.appsmith.server.services.FeatureFlagService; +import com.fasterxml.jackson.databind.ObjectMapper; +import io.micrometer.observation.ObservationRegistry; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; +import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +import java.util.ArrayList; +import java.util.List; + +import static java.lang.Boolean.FALSE; +import static java.lang.Boolean.TRUE; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +public class OnLoadExecutablesUtilCEImplTest { + + @Mock + private AstService astService; + + @Mock + private ObjectMapper objectMapper; + + @Mock + private ExecutableOnLoadService executableOnLoadService; + + @Mock + private ObservationRegistry observationRegistry; + + @Mock + private ObservationHelperImpl observationHelper; + + @Mock + private FeatureFlagService featureFlagService; + + private OnLoadExecutablesUtilCEImpl onLoadExecutablesUtilCE; + + @BeforeEach + public void setUp() { + onLoadExecutablesUtilCE = new OnLoadExecutablesUtilCEImpl( + astService, + objectMapper, + executableOnLoadService, + observationRegistry, + observationHelper, + featureFlagService); + + onLoadExecutablesUtilCE = spy(new OnLoadExecutablesUtilCEImpl( + astService, + objectMapper, + executableOnLoadService, + observationRegistry, + observationHelper, + featureFlagService)); + + ObservationRegistry.ObservationConfig mockObservationConfig = + Mockito.mock(ObservationRegistry.ObservationConfig.class); + Mockito.when(observationRegistry.observationConfig()).thenReturn(mockObservationConfig); + } + + @Test + public void testUpdateExecutablesRunBehaviour_WhenFeatureFlagEnabled_SetsRunBehaviourToAutomatic() { + // Setup + String creatorId = "testCreatorId"; + CreatorContextType creatorType = CreatorContextType.PAGE; + List executableUpdatesRef = new ArrayList<>(); + List messagesRef = new ArrayList<>(); + + // Create test executables + List onLoadExecutables = createTestExecutables("newOnLoad1", "newOnLoad2"); + List existingOnLoadExecutables = createTestExecutables("existingOnLoad1"); + List allExecutables = new ArrayList<>(); + allExecutables.addAll(onLoadExecutables); + allExecutables.addAll(existingOnLoadExecutables); + allExecutables.add(createTestExecutable("turnedOff1", RunBehaviourEnum.ON_PAGE_LOAD)); + + // Mock behavior + when(featureFlagService.check(FeatureFlagEnum.release_reactive_actions_enabled)) + .thenReturn(Mono.just(TRUE)); + + when(executableOnLoadService.getAllExecutablesByCreatorIdFlux(anyString())) + .thenReturn(Flux.fromIterable(allExecutables)); + + when(executableOnLoadService.updateUnpublishedExecutable(anyString(), any(ActionDTO.class))) + .thenAnswer(invocation -> Mono.just(invocation.getArgument(1))); + + // Execute + Mono result = onLoadExecutablesUtilCE.updateExecutablesRunBehaviour( + onLoadExecutables, creatorId, executableUpdatesRef, messagesRef, creatorType); + + // Verify + StepVerifier.create(result).expectNext(TRUE).verifyComplete(); + + // Verify that executables were updated with AUTOMATIC run behavior + verify(executableOnLoadService, times(2)) + .updateUnpublishedExecutable( + anyString(), + Mockito.argThat(executable -> executable.getRunBehaviour() == RunBehaviourEnum.AUTOMATIC)); + } + + @Test + public void testUpdateExecutablesRunBehaviour_WhenFeatureFlagDisabled_SetsRunBehaviourToOnPageLoad() { + // Setup + String creatorId = "testCreatorId"; + CreatorContextType creatorType = CreatorContextType.PAGE; + List executableUpdatesRef = new ArrayList<>(); + List messagesRef = new ArrayList<>(); + + // Create test executables + List onLoadExecutables = createTestExecutables("newOnLoad1", "newOnLoad2"); + List existingOnLoadExecutables = createTestExecutables("existingOnLoad1"); + List allExecutables = new ArrayList<>(); + allExecutables.addAll(onLoadExecutables); + allExecutables.addAll(existingOnLoadExecutables); + allExecutables.add(createTestExecutable("turnedOff1", RunBehaviourEnum.ON_PAGE_LOAD)); + + // Mock behavior + when(featureFlagService.check(FeatureFlagEnum.release_reactive_actions_enabled)) + .thenReturn(Mono.just(FALSE)); + + when(executableOnLoadService.getAllExecutablesByCreatorIdFlux(anyString())) + .thenReturn(Flux.fromIterable(allExecutables)); + + when(executableOnLoadService.updateUnpublishedExecutable(anyString(), any(ActionDTO.class))) + .thenAnswer(invocation -> Mono.just(invocation.getArgument(1))); + + // Execute + Mono result = onLoadExecutablesUtilCE.updateExecutablesRunBehaviour( + onLoadExecutables, creatorId, executableUpdatesRef, messagesRef, creatorType); + + // Verify + StepVerifier.create(result).expectNext(TRUE).verifyComplete(); + + // Verify that executables were updated with ON_PAGE_LOAD run behavior + verify(executableOnLoadService, times(2)) + .updateUnpublishedExecutable( + anyString(), + Mockito.argThat(executable -> executable.getRunBehaviour() == RunBehaviourEnum.ON_PAGE_LOAD)); + } + + @Test + public void testUpdateExecutablesRunBehaviour_WhenUserSetOnLoadIsTrue_DoesNotUpdateExecutable() { + // Setup + String creatorId = "testCreatorId"; + CreatorContextType creatorType = CreatorContextType.PAGE; + List executableUpdatesRef = new ArrayList<>(); + List messagesRef = new ArrayList<>(); + + // Create test executables + List onLoadExecutables = createTestExecutables("newOnLoad1", "userSetOnLoad1"); + + // Create an executable with userSetOnLoad = true + ActionDTO userSetExecutable = createTestExecutable("userSetOnLoad1", RunBehaviourEnum.MANUAL); + userSetExecutable.setUserSetOnLoad(TRUE); + + List allExecutables = new ArrayList<>(); + allExecutables.add(createTestExecutable("newOnLoad1", RunBehaviourEnum.MANUAL)); + allExecutables.add(userSetExecutable); + + // Mock behavior + when(featureFlagService.check(FeatureFlagEnum.release_reactive_actions_enabled)) + .thenReturn(Mono.just(TRUE)); + + when(executableOnLoadService.getAllExecutablesByCreatorIdFlux(anyString())) + .thenReturn(Flux.fromIterable(allExecutables)); + + when(executableOnLoadService.updateUnpublishedExecutable(anyString(), any(ActionDTO.class))) + .thenAnswer(invocation -> Mono.just(invocation.getArgument(1))); + + // Execute + Mono result = onLoadExecutablesUtilCE.updateExecutablesRunBehaviour( + onLoadExecutables, creatorId, executableUpdatesRef, messagesRef, creatorType); + + // Verify + StepVerifier.create(result).expectNext(TRUE).verifyComplete(); + + // Verify that only one executable was updated (the one with userSetOnLoad = false) + verify(executableOnLoadService, times(1)).updateUnpublishedExecutable(anyString(), any(ActionDTO.class)); + + // Verify that the executable with userSetOnLoad = true was not updated + verify(executableOnLoadService, times(0)) + .updateUnpublishedExecutable(eq(userSetExecutable.getId()), any(ActionDTO.class)); + } + + @Test + public void testUpdateExecutablesRunBehaviour_WhenNoExecutablesToUpdate_ReturnsFalse() { + // Setup + String creatorId = "testCreatorId"; + CreatorContextType creatorType = CreatorContextType.PAGE; + List executableUpdatesRef = new ArrayList<>(); + List messagesRef = new ArrayList<>(); + + // Create empty lists + List onLoadExecutables = new ArrayList<>(); + List existingOnLoadExecutables = new ArrayList<>(); + List allExecutables = new ArrayList<>(); + + // Mock behavior + when(executableOnLoadService.getAllExecutablesByCreatorIdFlux(anyString())) + .thenReturn(Flux.fromIterable(allExecutables)); + + when(featureFlagService.check(FeatureFlagEnum.release_reactive_actions_enabled)) + .thenReturn(Mono.just(TRUE)); + + // Execute + Mono result = onLoadExecutablesUtilCE.updateExecutablesRunBehaviour( + onLoadExecutables, creatorId, executableUpdatesRef, messagesRef, creatorType); + + // Verify + StepVerifier.create(result).expectNext(FALSE).verifyComplete(); + + // Verify that no executables were updated + verify(executableOnLoadService, times(0)).updateUnpublishedExecutable(anyString(), any(ActionDTO.class)); + } + + // Helper methods to create test executables + private List createTestExecutables(String... names) { + List executables = new ArrayList<>(); + for (String name : names) { + executables.add(createTestExecutable(name, RunBehaviourEnum.MANUAL)); + } + return executables; + } + + private ActionDTO createTestExecutable(String name, RunBehaviourEnum runBehaviour) { + ActionDTO actionDTO = new ActionDTO(); + actionDTO.setId(name + "Id"); + actionDTO.setName(name); + actionDTO.setRunBehaviour(runBehaviour); + actionDTO.setUserSetOnLoad(FALSE); + return actionDTO; + } +}