From c4d3d535a1e4bd6032bc33792b6d893255e85cea Mon Sep 17 00:00:00 2001 From: Shrikant Kandula Date: Mon, 18 May 2020 12:13:54 +0000 Subject: [PATCH] Rename `isExecuteOnLoad` to `executeOnLoad`. The `is` prefix apparently makes Spring unhappy. --- .../com/appsmith/server/domains/Action.java | 2 + .../server/repositories/ActionRepository.java | 6 +- .../server/services/ActionService.java | 2 +- .../server/services/ActionServiceImpl.java | 18 ++- .../services/LayoutActionServiceImpl.java | 38 ++---- .../server/services/LayoutServiceTest.java | 113 ++++++++++++++++++ 6 files changed, 148 insertions(+), 31 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Action.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Action.java index 41a538ab37..3a250e66dd 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Action.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Action.java @@ -33,6 +33,8 @@ public class Action extends BaseDomain { PluginType pluginType; + Boolean executeOnLoad; + @JsonProperty(access = JsonProperty.Access.READ_ONLY) Boolean isValid; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ActionRepository.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ActionRepository.java index e1cb852f8a..c1c4d8a263 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ActionRepository.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ActionRepository.java @@ -16,5 +16,9 @@ public interface ActionRepository extends BaseRepository { Flux findByPageId(String pageId); - Flux findDistinctActionsByNameInAndPageIdAndActionConfiguration_HttpMethod(Set names, String pageId, String httpMethod); + Flux findDistinctActionsByNameInAndPageIdAndActionConfiguration_HttpMethod( + Set names, String pageId, String httpMethod); + + Flux findDistinctActionsByNameInAndPageIdAndExecuteOnLoadTrue( + Set names, String pageId); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionService.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionService.java index 813a807408..9bb7c4d279 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionService.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionService.java @@ -17,7 +17,7 @@ public interface ActionService extends CrudService { Mono findByNameAndPageId(String name, String pageId); - Flux findDistinctRestApiActionsByNameInAndPageIdAndHttpMethod(Set names, String pageId, String httpMethod); + Flux findOnLoadActionsInPage(Set names, String pageId); Mono validateAndSaveActionToRepository(Action action); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java index 83adb7bd26..75a0de96a7 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java @@ -464,9 +464,23 @@ public class ActionServiceImpl extends BaseService findDistinctRestApiActionsByNameInAndPageIdAndHttpMethod(Set names, String pageId, String httpMethod) { - return repository.findDistinctActionsByNameInAndPageIdAndActionConfiguration_HttpMethod(names, pageId, httpMethod); + public Flux findOnLoadActionsInPage(Set names, String pageId) { + final Flux getApiActions = repository + .findDistinctActionsByNameInAndPageIdAndActionConfiguration_HttpMethod(names, pageId, "GET"); + + final Flux explicitOnLoadActions = repository + .findDistinctActionsByNameInAndPageIdAndExecuteOnLoadTrue(names, pageId); + + return getApiActions.concatWith(explicitOnLoadActions); } /** diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java index 58f7d2229a..3bf5435952 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java @@ -22,7 +22,6 @@ import org.springframework.stereotype.Service; import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; -import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import java.util.ArrayList; @@ -75,8 +74,6 @@ public class LayoutActionServiceImpl implements LayoutActionService { @Override public Mono updateLayout(String pageId, String layoutId, Layout layout) { - String dslString = ""; - JSONObject dsl = layout.getDsl(); if (dsl == null) { // There is no DSL here. No need to process anything. Return as is. @@ -84,11 +81,12 @@ public class LayoutActionServiceImpl implements LayoutActionService { } // Convert the DSL into a String + String dslString; try { dslString = objectMapper.writeValueAsString(dsl); } catch (JsonProcessingException e) { log.debug("Exception caught during conversion of DSL Json object to String. ", e); - Mono.error(new AppsmithException(AppsmithError.JSON_PROCESSING_ERROR, e.getMessage())); + return Mono.error(new AppsmithException(AppsmithError.JSON_PROCESSING_ERROR, e.getMessage())); } Set widgetNames = new HashSet<>(); @@ -109,9 +107,8 @@ public class LayoutActionServiceImpl implements LayoutActionService { return dynamicBindingNames; }); - List> onLoadActionsList = new ArrayList<>(); Mono>> onLoadActionsMono = dynamicBindingNamesMono - .flatMap(dynamicBindingNames -> findOnLoadActionsInPage(onLoadActionsList, dynamicBindingNames, pageId)); + .flatMap(dynamicBindingNames -> findOnLoadActionsInPage(dynamicBindingNames, pageId)); return pageService.findByIdAndLayoutsId(pageId, layoutId) .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.PAGE_ID + " or " + FieldName.LAYOUT_ID))) @@ -155,20 +152,22 @@ public class LayoutActionServiceImpl implements LayoutActionService { }); } + public Mono>> findOnLoadActionsInPage(Set dynamicBindingNames, String pageId) { + return findOnLoadActionsInPage(new ArrayList<>(), dynamicBindingNames, pageId); + } + private Mono>> findOnLoadActionsInPage(List> onLoadActions, Set dynamicBindingNames, String pageId) { if (dynamicBindingNames == null || dynamicBindingNames.isEmpty()) { return Mono.just(onLoadActions); } Set bindingNames = new HashSet<>(); - return findRestApiActionsByPageIdAndHTTPMethodGET(dynamicBindingNames, pageId) + return actionService.findOnLoadActionsInPage(dynamicBindingNames, pageId) .map(action -> { - if (action.getJsonPathKeys() != null) { + if (!CollectionUtils.isEmpty(action.getJsonPathKeys())) { for (String mustacheKey : action.getJsonPathKeys()) { extractWordsAndAddToSet(bindingNames, mustacheKey); } - if (bindingNames.contains(action.getName())) { - bindingNames.remove(action.getName()); - } + bindingNames.remove(action.getName()); } DslActionDTO newAction = new DslActionDTO(); newAction.setId(action.getId()); @@ -182,8 +181,7 @@ public class LayoutActionServiceImpl implements LayoutActionService { }) .collect(toSet()) .flatMap(actions -> { - HashSet onLoadSet = new HashSet<>(); - onLoadSet.addAll(actions); + HashSet onLoadSet = new HashSet<>(actions); // If the resultant set of actions is empty, don't add it to the array list. if (!onLoadSet.isEmpty()) { @@ -210,20 +208,6 @@ public class LayoutActionServiceImpl implements LayoutActionService { } } - /** - * Given a list of names of actions (nodes) and pageId, it hits the database and returns all the actions matching - * this criteria of name and pageId with http method 'GET' - * - * @param nodes - * @param pageId - * @return - */ - Flux findRestApiActionsByPageIdAndHTTPMethodGET(Set nodes, String pageId) { - - return actionService - .findDistinctRestApiActionsByNameInAndPageIdAndHttpMethod(nodes, pageId, "GET"); - } - @Override public Mono moveAction(ActionMoveDTO actionMoveDTO) { Action action = actionMoveDTO.getAction(); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutServiceTest.java index 8cf4bb82e4..456c2b2c4e 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutServiceTest.java @@ -1,9 +1,12 @@ package com.appsmith.server.services; +import com.appsmith.external.models.ActionConfiguration; import com.appsmith.server.constants.FieldName; +import com.appsmith.server.domains.Action; import com.appsmith.server.domains.Application; import com.appsmith.server.domains.Layout; import com.appsmith.server.domains.Page; +import com.appsmith.server.dtos.DslActionDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import lombok.extern.slf4j.Slf4j; @@ -14,13 +17,19 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.http.HttpMethod; import org.springframework.security.test.context.support.WithUserDetails; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringRunner; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.assertThat; @@ -41,6 +50,9 @@ public class LayoutServiceTest { @Autowired LayoutActionService layoutActionService; + @Autowired + ActionService actionService; + Mono layoutMono; Mono applicationMono; @@ -179,6 +191,107 @@ public class LayoutServiceTest { .verifyComplete(); } + @Test + @WithUserDetails(value = "api_user") + public void getActionsExecuteOnLoad() { + Mono testMono = pageService + .findByName("validPageName") + .flatMap(page1 -> { + List> monos = new ArrayList<>(); + + Action action = new Action(); + action.setName("aGetAction"); + action.setActionConfiguration(new ActionConfiguration()); + action.getActionConfiguration().setHttpMethod(HttpMethod.GET); + action.setPageId(page1.getId()); + monos.add(actionService.create(action)); + + action = new Action(); + action.setName("aPostAction"); + action.setActionConfiguration(new ActionConfiguration()); + action.getActionConfiguration().setHttpMethod(HttpMethod.POST); + action.setPageId(page1.getId()); + monos.add(actionService.create(action)); + + action = new Action(); + action.setName("aPostActionWithAutoExec"); + action.setActionConfiguration(new ActionConfiguration()); + action.getActionConfiguration().setHttpMethod(HttpMethod.POST); + action.getActionConfiguration().setBody( + "this won't be auto-executed: {{aPostSecondaryAction.data}}, but this one will be: {{aPostTertiaryAction.data}}."); + action.setJsonPathKeys(Set.of("aPostSecondaryAction.data", "aPostTertiaryAction.data")); + action.setPageId(page1.getId()); + action.setExecuteOnLoad(true); + monos.add(actionService.create(action)); + + action = new Action(); + action.setName("aPostSecondaryAction"); + action.setActionConfiguration(new ActionConfiguration()); + action.getActionConfiguration().setHttpMethod(HttpMethod.POST); + action.setPageId(page1.getId()); + monos.add(actionService.create(action)); + + action = new Action(); + action.setName("aPostTertiaryAction"); + action.setActionConfiguration(new ActionConfiguration()); + action.getActionConfiguration().setHttpMethod(HttpMethod.POST); + action.setPageId(page1.getId()); + action.setExecuteOnLoad(true); + monos.add(actionService.create(action)); + + action = new Action(); + action.setName("aDeleteAction"); + action.setActionConfiguration(new ActionConfiguration()); + action.getActionConfiguration().setHttpMethod(HttpMethod.DELETE); + action.setPageId(page1.getId()); + monos.add(actionService.create(action)); + + return Mono.zip(monos, objects -> page1); + }) + .zipWhen(page1 -> { + Layout layout = new Layout(); + + JSONObject obj = new JSONObject(Map.of( + "key", "value" + )); + layout.setDsl(obj); + + return layoutService.createLayout(page1.getId(), layout); + }) + .flatMap(tuple2 -> { + final Page page1 = tuple2.getT1(); + final Layout layout = tuple2.getT2(); + + Layout newLayout = new Layout(); + + JSONObject obj = new JSONObject(Map.of( + "key", "value-updated", + "another", "Hello people of the {{input1.text}} planet!", + "dynamicGet", "some dynamic {{aGetAction.data}}", + "dynamicPost", "some dynamic {{aPostAction.data}}", + "dynamicPostWithAutoExec", "some dynamic {{aPostActionWithAutoExec.data}}", + "dynamicDelete", "some dynamic {{aDeleteAction.data}}" + )); + newLayout.setDsl(obj); + + return layoutActionService.updateLayout(page1.getId(), layout.getId(), newLayout); + }); + + StepVerifier + .create(testMono) + .assertNext(layout -> { + assertThat(layout).isNotNull(); + assertThat(layout.getId()).isNotNull(); + assertThat(layout.getDsl().get("key")).isEqualTo("value-updated"); + assertThat(layout.getLayoutOnLoadActions()).hasSize(2); + assertThat(layout.getLayoutOnLoadActions().get(0).stream().map(DslActionDTO::getName).collect(Collectors.toSet())) + .hasSameElementsAs(Set.of("aPostTertiaryAction")); + assertThat(layout.getLayoutOnLoadActions().get(1).stream().map(DslActionDTO::getName).collect(Collectors.toSet())) + .hasSameElementsAs(Set.of("aGetAction", "aPostActionWithAutoExec")); + }) + .verifyComplete(); + } + @After public void purgePages() { pageService.deleteAll();