Protei/PromucFlow_constructor
2
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix: fix page load action execution order (#10252)

Browse Source 
* This PR fixes the page load action execution order when the actions have been set to run on page load explicitly via the settings tab by the user and its data has not been referenced in any other widget or action. e.g.
  - create action1 and action2.
  - make action2 dependent on action1 by adding {{action1.data}} in action2's body.
  - set both action1 and action2 to run on page load via settings tab. Do not reference action1 and action2 data in any other widget or action.
This commit is contained in:
Sumit Kumar 2022-01-11 21:43:54 +05:30 committed by GitHub
parent a0aa41145b
commit 7306967d0b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 157 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java
View File

@ -210,7 +210,7 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
if (newAction.getPublishedAction() != null) {
action = newAction.getPublishedAction();
} else {
// We are trying to fetch published action but it doesnt exist because the action hasn't been published yet
// We are trying to fetch published action but it doesn't exist because the action hasn't been published yet
return Mono.empty();
}
} else {

30
app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/PageLoadActionsUtilCEImpl.java
View File

@ -163,7 +163,8 @@ public class PageLoadActionsUtilCEImpl implements PageLoadActionsUtilCE {
Set<String> actionNames = tuple.getT1();
DirectedAcyclicGraph<String, DefaultEdge> graph = tuple.getT2();
return computeOnPageLoadActionsSchedulingOrder(graph, onPageLoadActionSet, actionNames);
return computeOnPageLoadActionsSchedulingOrder(graph, onPageLoadActionSet, actionNames,
explicitUserSetOnLoadActions);
})
.map(onPageLoadActionsSchedulingOrder -> {
// Find all explicitly turned on actions which haven't found their way into the scheduling order
@ -517,13 +518,15 @@ public class PageLoadActionsUtilCEImpl implements PageLoadActionsUtilCE {
* Breadth First level by level traversal is used to compute each set of such independent actions.
*
* @param dag : The DAG graph containing all the edges representing dependencies between appsmith entities in the page.
* @param pageLoadActionSet
* @param onPageLoadActionSet
* @param actionNames : All the action names for the page
* @return
*/
private List<Set<String>> computeOnPageLoadActionsSchedulingOrder(DirectedAcyclicGraph<String, DefaultEdge> dag,
Set<String> onPageLoadActionSet,
Set<String> actionNames) {
Set<String> actionNames,
Set<String> explicitUserSetOnLoadActions) {
Map<String, Integer> pageLoadActionAndLevelMap = new HashMap<>();
List<Set<String>> onPageLoadActions = new ArrayList<>();
@ -544,7 +547,8 @@ public class PageLoadActionsUtilCEImpl implements PageLoadActionsUtilCE {
onPageLoadActions.add(new HashSet<>());
}
Set<String> actionsFromBinding = actionCandidatesForPageLoadFromBinding(actionNames, vertex, pageLoadActionAndLevelMap, onPageLoadActions);
Set<String> actionsFromBinding = actionCandidatesForPageLoadFromBinding(actionNames, vertex,
pageLoadActionAndLevelMap, onPageLoadActions, explicitUserSetOnLoadActions);
onPageLoadActions.get(level).addAll(actionsFromBinding);
for (String action : actionsFromBinding) {
pageLoadActionAndLevelMap.put(action, level);
@ -908,7 +912,8 @@ public class PageLoadActionsUtilCEImpl implements PageLoadActionsUtilCE {
private Set<String> actionCandidatesForPageLoadFromBinding(Set<String> allActionNames,
String vertex,
Map<String, Integer> pageLoadActionsLevelMap,
List<Set<String>> existingPageLoadActions) {
List<Set<String>> existingPageLoadActions,
Set<String> explicitUserSetOnLoadActions) {
Set<String> onPageLoadCandidates = new HashSet<>();
@ -921,10 +926,21 @@ public class PageLoadActionsUtilCEImpl implements PageLoadActionsUtilCE {
Boolean isCandidateForPageLoad = TRUE;
// Only add it for page load if it is not a function call. Aka the data
// of this call is being referred to in the binding.
/**
* Add action for page load if:
* o it has been explicitly set to run on page load by the user (even if its data is not
* referenced in any widget or action)
* o or, it is not a function call i.e. the data of this call is being referred to in the binding.
*/
String validBinding;
if (explicitUserSetOnLoadActions.contains(entity)) {
validBinding = entity + "." + "actionConfiguration";
}
else {
validBinding = entity + "." + "data";
}
String validBinding = entity + "." + "data";
if (!vertex.contains(validBinding)) {
isCandidateForPageLoad = FALSE;
}

134
app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java
View File

@ -48,10 +48,12 @@ import org.mockito.Mockito;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.boot.test.mock.mockito.SpyBean;
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.Flux;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
import reactor.util.function.Tuple2;
@ -77,7 +79,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
@Slf4j
@DirtiesContext
public class LayoutActionServiceTest {
@Autowired
@SpyBean
NewActionService newActionService;
@Autowired
@ -1384,4 +1386,134 @@ public class LayoutActionServiceTest {
})
.verifyComplete();
}
/**
* This method tests the following case:
* o create action1 and action2.
* o make action2 dependent on action1 by adding {{action1.data}} in action2's body.
* o set both action1 and action2 to run on page load via settings tab. Do not reference action1 and action2 data
* in any other widget or action.
* @throws JsonProcessingException
*/
@Test
@WithUserDetails(value = "api_user")
public void testExecuteOnPageLoadOrderWhenAllActionsAreOnlyExplicitlySetToExecute() throws JsonProcessingException {
Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())).thenReturn(Mono.just(new MockPluginExecutor()));
// Configure action1
ActionDTO action1 = new ActionDTO();
action1.setName("firstAction");
action1.setPageId(testPage.getId());
ActionConfiguration actionConfiguration1 = new ActionConfiguration();
actionConfiguration1.setHttpMethod(HttpMethod.GET);
action1.setActionConfiguration(actionConfiguration1);
action1.setExecuteOnLoad(true);
action1.setDatasource(datasource);
// Configure action2
ActionDTO action2 = new ActionDTO();
action2.setName("secondAction");
action2.setPageId(testPage.getId());
ActionConfiguration actionConfiguration2 = new ActionConfiguration();
actionConfiguration2.setHttpMethod(HttpMethod.GET);
actionConfiguration2.setBody("{{ firstAction.data }}"); // make action2 dependent on action1
action2.setActionConfiguration(actionConfiguration2);
action2.setDynamicBindingPathList(List.of(new Property("body", null)));
action2.setExecuteOnLoad(true);
action2.setDatasource(datasource);
JSONObject parentDsl = new JSONObject(objectMapper.readValue(DEFAULT_PAGE_LAYOUT, new TypeReference<HashMap<String, Object>>() {
}));
Layout layout = testPage.getLayouts().get(0);
layout.setDsl(parentDsl);
ActionDTO createdAction1 = layoutActionService.createSingleAction(action1).block(); // create action1
createdAction1.setExecuteOnLoad(true); // this can only be set to true post action creation.
NewAction newAction1 = new NewAction();
newAction1.setUnpublishedAction(createdAction1);
newAction1.setDefaultResources(createdAction1.getDefaultResources());
ActionDTO createdAction2 = layoutActionService.createSingleAction(action2).block(); // create action2
createdAction2.setExecuteOnLoad(true); // this can only be set to true post action creation.
NewAction newAction2 = new NewAction();
newAction2.setUnpublishedAction(createdAction2);
newAction2.setDefaultResources(createdAction2.getDefaultResources());
NewAction[] newActionArray = new NewAction[2];
newActionArray[0] = newAction1;
newActionArray[1] = newAction2;
Flux<NewAction> newActionFlux = Flux.fromArray(newActionArray);
Mockito.when(newActionService.findUnpublishedOnLoadActionsExplicitSetByUserInPage(Mockito.any())).thenReturn(newActionFlux);
Mono<LayoutDTO> updateLayoutMono = layoutActionService.updateLayout(testPage.getId(), layout.getId(), layout);
StepVerifier.create(updateLayoutMono)
.assertNext(updatedLayout -> {
assertThat(updatedLayout.getLayoutOnLoadActions().size()).isEqualTo(2);
// Assert that both the actions don't belong to the same set. They should be run iteratively.
DslActionDTO actionDTO1 = updatedLayout.getLayoutOnLoadActions().get(0).iterator().next();
assertThat(actionDTO1.getName()).isEqualTo("firstAction");
DslActionDTO actionDTO2 = updatedLayout.getLayoutOnLoadActions().get(1).iterator().next();
assertThat(actionDTO2.getName()).isEqualTo("secondAction");
})
.verifyComplete();
}
/**
* This method tests the following scenario:
* o create `action1`.
* o set `action1` to run on page load via settings tab.
* o bind `{{action1.data}}` in one of the widget fields.
*/
@Test
@WithUserDetails(value = "api_user")
public void testPageLoadActionWhenSetBothWaysExplicitlyAndImplicitlyViaWidget() {
Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())).thenReturn(Mono.just(new MockPluginExecutor()));
ActionDTO action1 = new ActionDTO();
action1.setName("firstAction");
action1.setPageId(testPage.getId());
ActionConfiguration actionConfiguration1 = new ActionConfiguration();
actionConfiguration1.setHttpMethod(HttpMethod.GET);
action1.setActionConfiguration(actionConfiguration1);
action1.setDatasource(datasource);
JSONObject dsl = new JSONObject();
dsl.put("widgetName", "firstWidget");
JSONArray temp = new JSONArray();
temp.addAll(List.of(new JSONObject(Map.of("key", "testField"))));
dsl.put("dynamicBindingPathList", temp);
dsl.put("testField", "{{ firstAction.data }}");
Layout layout = testPage.getLayouts().get(0);
layout.setDsl(dsl);
ActionDTO createdAction1 = layoutActionService.createSingleAction(action1).block();
createdAction1.setExecuteOnLoad(true); // this can only be set to true post action creation.
createdAction1.setUserSetOnLoad(true);
NewAction newAction1 = new NewAction();
newAction1.setUnpublishedAction(createdAction1);
newAction1.setDefaultResources(createdAction1.getDefaultResources());
NewAction[] newActionArray = new NewAction[1];
newActionArray[0] = newAction1;
Flux<NewAction> newActionFlux = Flux.fromArray(newActionArray);
Mockito.when(newActionService.findUnpublishedOnLoadActionsExplicitSetByUserInPage(Mockito.any())).thenReturn(newActionFlux);
Mono<LayoutDTO> updateLayoutMono = layoutActionService.updateLayout(testPage.getId(), layout.getId(), layout);
StepVerifier.create(updateLayoutMono)
.assertNext(updatedLayout -> {
assertThat(updatedLayout.getLayoutOnLoadActions().size()).isEqualTo(1);
// Assert that both the actions don't belong to the same set. They should be run iteratively.
DslActionDTO actionDTO1 = updatedLayout.getLayoutOnLoadActions().get(0).iterator().next();
assertThat(actionDTO1.getName()).isEqualTo("firstAction");
})
.verifyComplete();
}
}