From e8cb73dc683ab484e2c5567e15b22ef486766ad5 Mon Sep 17 00:00:00 2001 From: Rishabh Rathod Date: Fri, 29 Nov 2024 17:16:19 +0530 Subject: [PATCH] fix: Reduce RTS calls in the updateLayout flow (#37127) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description In this PR, we update the code to send all the bindings to RTS together instead of calling it for each property binding. It was observed that the calls were earlier around 40k for the test application. This change leads to optimisation of performance for - updateLayout call ( when updating anything in the DSL ) - query/API action update - JS action update Fixes #37055 ## Tests on DP Deploy-Preview-URL: https://ce-37127.dp.appsmith.com/ - [x] Multiple widgets with the same bindings impacting the sequence of onPageLoadActions - onPageLoadActions update as expected when - [x] DSL is updated - [x] Query is updated - [x] JS Object is updated EE PR - https://github.com/appsmithorg/appsmith-ee/pull/5642 ## Automation /ok-to-test tags="@tag.All" ### :mag: Cypress test results > [!TIP] > ๐ŸŸข ๐ŸŸข ๐ŸŸข All cypress tests have passed! ๐ŸŽ‰ ๐ŸŽ‰ ๐ŸŽ‰ > Workflow run: > Commit: 07b68d4f0663ff336807223c93feceb1264741df > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Fri, 29 Nov 2024 11:44:31 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Enhanced retrieval of possible entity references and their relationships with a new method. - Added new test cases to validate action execution on page load and dynamic binding handling. - **Bug Fixes** - Improved error handling and validation for executable references. - Enhanced assertions to ensure correct execution of layout actions. - **Tests** - Added a test to validate layout actions with multiple widgets referencing the same action. - Refactored existing tests for better readability and maintainability. - Expanded test coverage for layout updates and dynamic binding scenarios. --------- Co-authored-by: โ€œsneha122โ€ <โ€œsneha@appsmith.comโ€> --- .../constants/spans/ce/OnLoadSpanCE.java | 1 - .../internal/OnLoadExecutablesUtilCEImpl.java | 140 +++++++++--- .../services/ActionCollectionServiceTest.java | 213 +++++++++++------- .../server/services/LayoutServiceTest.java | 83 ++++++- 4 files changed, 313 insertions(+), 124 deletions(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/OnLoadSpanCE.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/OnLoadSpanCE.java index 138aa40f93f2..1859a37ef21e 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/OnLoadSpanCE.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/OnLoadSpanCE.java @@ -8,7 +8,6 @@ public class OnLoadSpanCE { APPSMITH_SPAN_PREFIX + "getAllExecutablesByCreatorIdFlux"; public static final String EXECUTABLE_NAME_TO_EXECUTABLE_MAP = APPSMITH_SPAN_PREFIX + "executableNameToExecutableMap"; - public static final String EXECUTABLE_IN_CREATOR_CONTEXT = APPSMITH_SPAN_PREFIX + "executablesInCreatorContext"; public static final String ADD_DIRECTLY_REFERENCED_EXECUTABLES_TO_GRAPH = APPSMITH_SPAN_PREFIX + "addDirectlyReferencedExecutablesToGraph"; public static final String UPDATE_EXECUTABLE_SELF_REFERENCING_PATHS = 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 08c6840bc62c..ef535c21694f 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 @@ -524,9 +524,8 @@ private Mono> getPossibleEntityReferences( Set bindingsInDsl) { // We want to be finding both type of references final int entityTypes = EXECUTABLE_ENTITY_REFERENCES | WIDGET_ENTITY_REFERENCES; - return executableNameToExecutableMono - .zipWith(getPossibleEntityParentsMap(bindings, entityTypes, evalVersion)) + .zipWith(getPossibleEntityParentsMap(new ArrayList<>(bindings), entityTypes, evalVersion)) .map(tuple -> { Map executableMap = tuple.getT1(); // For each binding, here we receive a set of possible references to global entities @@ -584,6 +583,79 @@ private Mono> getPossibleEntityReferences( }); } + private Mono>> getPossibleEntityReferencesMap( + Mono> executableNameToExecutableMono, + List bindings, + int evalVersion, + Set bindingsInDsl) { + // We want to be finding both type of references + final int entityTypes = EXECUTABLE_ENTITY_REFERENCES | WIDGET_ENTITY_REFERENCES; + + return executableNameToExecutableMono + .zipWith(getPossibleEntityParentsMap(bindings, entityTypes, evalVersion)) + .map(tuple -> { + Map executableMap = tuple.getT1(); + // For each binding, here we receive a set of possible references to global entities + // At this point we're guaranteed that these references are made to possible variables, + // but we do not know if those entities exist in the global namespace yet + Map> bindingToPossibleParentMap = tuple.getT2(); + + Map> possibleEntitiesReferencesToBindingMap = new HashMap<>(); + + // From these references, we will try to validate executable references at this point + // Each identified node is already annotated with the expected type of entity we need to search for + bindingToPossibleParentMap.entrySet().stream().forEach(entry -> { + Set bindingsWithExecutableReference = new HashSet<>(); + String binding = entry.getKey(); + Set possibleEntitiesReferences = new HashSet<>(); + entry.getValue().stream().forEach(possibleParent -> { + // For each possible reference node, check if the reference was to an executable + Executable executable = executableMap.get(possibleParent.getValidEntityName()); + + if (executable != null) { + // If it was, and had been identified as the same type of executable as what exists in + // this app, + if (possibleParent + .getEntityReferenceType() + .equals(executable.getEntityReferenceType())) { + // Copy over some data from the identified executable, this ensures that we do not + // have + // to query the DB again later + possibleParent.setExecutable(executable); + bindingsWithExecutableReference.add(possibleParent); + // Only if this is not a direct JS function call, + // add it to a possible on page load executable call. + // This discards the following type: + // {{ JSObject1.func() }} + if (!TRUE.equals(possibleParent.getIsFunctionCall())) { + possibleEntitiesReferences.add(possibleParent); + } + // We're ignoring any reference that was identified as a widget but actually matched + // an executable + // We wouldn't have discarded JS collection names here, but this is just an + // optimization, so it's fine + } + } else { + // If the reference node was identified as a widget, directly add it as a possible + // reference + // Because we are not doing any validations for widget references at this point + if (EntityReferenceType.WIDGET.equals(possibleParent.getEntityReferenceType())) { + possibleEntitiesReferences.add(possibleParent); + } + } + + possibleEntitiesReferencesToBindingMap.put(binding, possibleEntitiesReferences); + }); + + if (!bindingsWithExecutableReference.isEmpty() && bindingsInDsl != null) { + bindingsInDsl.addAll(bindingsWithExecutableReference); + } + }); + + return possibleEntitiesReferencesToBindingMap; + }); + } + /** * This method is an abstraction that queries the ast service for possible global references as string values, * and then uses the mustache helper utility to classify these global references into possible types of EntityDependencyNodes @@ -594,9 +666,9 @@ private Mono> getPossibleEntityReferences( * @return A mono of a map of each of the provided binding values to the possible set of EntityDependencyNodes found in the binding */ private Mono>> getPossibleEntityParentsMap( - Set bindings, int types, int evalVersion) { + List bindings, int types, int evalVersion) { Flux>> findingToReferencesFlux = - astService.getPossibleReferencesFromDynamicBinding(new ArrayList<>(bindings), evalVersion); + astService.getPossibleReferencesFromDynamicBinding(bindings, evalVersion); return MustacheHelper.getPossibleEntityParentsMap(findingToReferencesFlux, types); } @@ -627,31 +699,45 @@ private Mono> addDirectlyReferencedExecutablesToGr Mono> executableNameToExecutableMapMono, Set executableBindingsInDslRef, int evalVersion) { - return Flux.fromIterable(widgetDynamicBindingsMap.entrySet()) - .flatMap(entry -> { - String widgetName = entry.getKey(); - // For each widget in the DSL that has a dynamic binding, - // we define an entity dependency node beforehand - // This will be a leaf node in the DAG that is constructed for on page load dependencies - EntityDependencyNode widgetDependencyNode = - new EntityDependencyNode(EntityReferenceType.WIDGET, widgetName, widgetName, null, null); - Set bindingsInWidget = entry.getValue(); - return getPossibleEntityReferences( - executableNameToExecutableMapMono, - bindingsInWidget, - evalVersion, - executableBindingsInDslRef) - .flatMapMany(Flux::fromIterable) - // Add dependencies of the executables found in the DSL in the graph - // We are ignoring the widget references at this point - // TODO: Possible optimization in the future - .flatMap(possibleEntity -> { + + Map> bindingToWidgetNodesMap = new HashMap<>(); + List allBindings = new ArrayList<>(); + + widgetDynamicBindingsMap.forEach((widgetName, bindingsInWidget) -> { + EntityDependencyNode widgetDependencyNode = + new EntityDependencyNode(EntityReferenceType.WIDGET, widgetName, widgetName, null, null); + + bindingsInWidget.forEach(binding -> { + bindingToWidgetNodesMap + .computeIfAbsent(binding, bindingKey -> new HashSet<>()) + .add(widgetDependencyNode); + allBindings.add(binding); + }); + }); + + Mono>> bindingToPossibleEntityMapMono = getPossibleEntityReferencesMap( + executableNameToExecutableMapMono, allBindings, evalVersion, executableBindingsInDslRef); + + return bindingToPossibleEntityMapMono + .flatMapMany(bindingToPossibleEntityMap -> Flux.fromIterable(bindingToPossibleEntityMap.entrySet())) + .flatMap(bindingEntry -> { + String binding = bindingEntry.getKey(); + Set possibleEntities = bindingEntry.getValue(); + + // Get all widget nodes associated with the binding + Set widgetDependencyNodes = + bindingToWidgetNodesMap.getOrDefault(binding, Set.of()); + + // Process each possibleEntity for the current binding + return Flux.fromIterable(possibleEntities).flatMap(possibleEntity -> Flux.fromIterable( + widgetDependencyNodes) // Iterate all associated widgets + .flatMap(widgetDependencyNode -> { if (getExecutableTypes().contains(possibleEntity.getEntityReferenceType())) { edgesRef.add(new ExecutableDependencyEdge(possibleEntity, widgetDependencyNode)); // This executable is directly referenced in the DSL. This executable is an ideal - // candidate - // for on page load + // candidate for on page load executablesUsedInDSLRef.add(possibleEntity.getValidEntityName()); + return updateExecutableSelfReferencingPaths(possibleEntity) .name(UPDATE_EXECUTABLE_SELF_REFERENCING_PATHS) .tap(Micrometer.observation(observationRegistry)) @@ -668,7 +754,7 @@ private Mono> addDirectlyReferencedExecutablesToGr .thenReturn(possibleEntity); } return Mono.just(possibleEntity); - }); + })); }) .collectList() .thenReturn(edgesRef); @@ -1134,7 +1220,7 @@ private Mono> addWidgetRelationshipToGraph( // This part will ensure that we are discovering widget to widget relationships. return Flux.fromIterable(widgetBindingMap.entrySet()) .flatMap(widgetBindingEntries -> getPossibleEntityParentsMap( - widgetBindingEntries.getValue(), entityTypes, evalVersion) + new ArrayList<>(widgetBindingEntries.getValue()), entityTypes, evalVersion) .map(possibleParentsMap -> { possibleParentsMap.entrySet().stream().forEach(entry -> { if (entry.getValue() == null || entry.getValue().isEmpty()) { diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java index 8c0471e7e36f..3feeac7facf7 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java @@ -1,5 +1,6 @@ package com.appsmith.server.services; +import com.appsmith.external.dtos.DslExecutableDTO; import com.appsmith.external.models.ActionConfiguration; import com.appsmith.external.models.ActionDTO; import com.appsmith.external.models.Datasource; @@ -64,6 +65,7 @@ import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.stream.Collectors; import static com.appsmith.server.acl.AclPermission.EXECUTE_ACTIONS; import static com.appsmith.server.acl.AclPermission.MANAGE_ACTIONS; @@ -710,42 +712,48 @@ public void testDeleteActionCollection_afterApplicationPublish_clearsActionColle .verifyComplete(); } - @Test - @WithUserDetails(value = "api_user") - public void - testUpdateUnpublishedActionCollection_withValidCollection_callsPageLayoutOnlyOnceAndAssertCyclicDependencyError() { - Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())).thenReturn(Mono.just(pluginExecutor)); - Mockito.when(pluginExecutor.getHintMessages(Mockito.any(), Mockito.any())) - .thenReturn(Mono.zip(Mono.just(new HashSet<>()), Mono.just(new HashSet<>()))); + private ActionDTO createAction(String actionName, String body, boolean isValid) { + ActionDTO testAction = new ActionDTO(); + testAction.setName(actionName); + testAction.setActionConfiguration(new ActionConfiguration()); + testAction.getActionConfiguration().setBody(body); + testAction.getActionConfiguration().setIsValid(isValid); + return testAction; + } + private ActionCollectionDTO createActionCollection(String collectionName, String body, PluginType pluginType) { ActionCollectionDTO actionCollectionDTO = new ActionCollectionDTO(); - actionCollectionDTO.setName("testCollection1"); + actionCollectionDTO.setName(collectionName); actionCollectionDTO.setPageId(testPage.getId()); actionCollectionDTO.setApplicationId(testApp.getId()); actionCollectionDTO.setWorkspaceId(workspaceId); actionCollectionDTO.setPluginId(datasource.getPluginId()); actionCollectionDTO.setVariables(List.of(new JSValue("test", "String", "test", true))); - actionCollectionDTO.setBody("collectionBody"); - actionCollectionDTO.setPluginType(PluginType.JS); + actionCollectionDTO.setBody(body); + actionCollectionDTO.setPluginType(pluginType); + return actionCollectionDTO; + } - // Create actions - ActionDTO action1 = new ActionDTO(); - action1.setName("testAction1"); - action1.setActionConfiguration(new ActionConfiguration()); - action1.getActionConfiguration().setBody("initial body"); - action1.getActionConfiguration().setIsValid(false); + private JSONArray createDynamicList(String key, String value) { + JSONArray temp2 = new JSONArray(); + temp2.add(new JSONObject(Map.of(key, value))); + return temp2; + } - ActionDTO action2 = new ActionDTO(); - action2.setName("testAction2"); - action2.setActionConfiguration(new ActionConfiguration()); - action2.getActionConfiguration().setBody("mockBody"); - action2.getActionConfiguration().setIsValid(false); + @Test + @WithUserDetails(value = "api_user") + public void + testUpdateUnpublishedActionCollection_withValidCollection_callsPageLayoutOnlyOnceAndAssertCyclicDependencyError() { + Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())).thenReturn(Mono.just(pluginExecutor)); + Mockito.when(pluginExecutor.getHintMessages(Mockito.any(), Mockito.any())) + .thenReturn(Mono.zip(Mono.just(new HashSet<>()), Mono.just(new HashSet<>()))); - ActionDTO action3 = new ActionDTO(); - action3.setName("testAction3"); - action3.setActionConfiguration(new ActionConfiguration()); - action3.getActionConfiguration().setBody("mockBody"); - action3.getActionConfiguration().setIsValid(false); + ActionCollectionDTO actionCollectionDTO = + createActionCollection("testCollection1", "collectionBody", PluginType.JS); + + ActionDTO action1 = createAction("testAction1", "initial body", false); + ActionDTO action2 = createAction("testAction2", "mockBody", false); + ActionDTO action3 = createAction("testAction3", "mockBody", false); actionCollectionDTO.setActions(List.of(action1, action2, action3)); @@ -753,12 +761,8 @@ public void testDeleteActionCollection_afterApplicationPublish_clearsActionColle ArrayList dslList = (ArrayList) layout.getDsl().get("children"); JSONObject tableDsl = (JSONObject) dslList.get(0); tableDsl.put("tableData", "{{testCollection1.testAction1.data}}"); - JSONArray temp2 = new JSONArray(); - temp2.add(new JSONObject(Map.of("key", "tableData"))); - tableDsl.put("dynamicBindingPathList", temp2); - JSONArray temp3 = new JSONArray(); - temp3.add(new JSONObject(Map.of("key", "tableData"))); - tableDsl.put("dynamicPropertyPathList", temp3); + tableDsl.put("dynamicBindingPathList", createDynamicList("key", "tableData")); + tableDsl.put("dynamicPropertyPathList", createDynamicList("key", "tableData")); layout.getDsl().put("widgetName", "MainContainer"); testPage.setLayouts(List.of(layout)); @@ -815,22 +819,10 @@ public void testDeleteActionCollection_afterApplicationPublish_clearsActionColle Mockito.when(pluginExecutor.getHintMessages(Mockito.any(), Mockito.any())) .thenReturn(Mono.zip(Mono.just(new HashSet<>()), Mono.just(new HashSet<>()))); - ActionCollectionDTO actionCollectionDTO = new ActionCollectionDTO(); - actionCollectionDTO.setName("testCollection1"); - actionCollectionDTO.setPageId(testPage.getId()); - actionCollectionDTO.setApplicationId(testApp.getId()); - actionCollectionDTO.setWorkspaceId(workspaceId); - actionCollectionDTO.setPluginId(datasource.getPluginId()); - actionCollectionDTO.setVariables(List.of(new JSValue("test", "String", "test", true))); - actionCollectionDTO.setBody("collectionBody"); - actionCollectionDTO.setPluginType(PluginType.JS); + ActionCollectionDTO actionCollectionDTO = + createActionCollection("testCollection1", "collectionBody", PluginType.JS); - // Create actions - ActionDTO action1 = new ActionDTO(); - action1.setName("testAction1"); - action1.setActionConfiguration(new ActionConfiguration()); - action1.getActionConfiguration().setBody("initial body"); - action1.getActionConfiguration().setIsValid(false); + ActionDTO action1 = createAction("testAction1", "initial body", false); actionCollectionDTO.setActions(List.of(action1)); // Create Js object @@ -840,18 +832,8 @@ public void testDeleteActionCollection_afterApplicationPublish_clearsActionColle assert createdActionCollectionDTO.getId() != null; String createdActionCollectionId = createdActionCollectionDTO.getId(); - // Update JS object to create an action with same name as previously created action - ActionDTO action2 = new ActionDTO(); - action2.setName("testAction1"); - action2.setActionConfiguration(new ActionConfiguration()); - action2.getActionConfiguration().setBody("mockBody"); - action2.getActionConfiguration().setIsValid(false); - - ActionDTO action3 = new ActionDTO(); - action3.setName("testAction2"); - action3.setActionConfiguration(new ActionConfiguration()); - action3.getActionConfiguration().setBody("mockBody"); - action3.getActionConfiguration().setIsValid(false); + ActionDTO action2 = createAction("testAction1", "mockBody", false); + ActionDTO action3 = createAction("testAction2", "mockBody", false); actionCollectionDTO.setActions( List.of(createdActionCollectionDTO.getActions().get(0), action2, action3)); @@ -875,22 +857,9 @@ public void testDeleteActionCollection_afterApplicationPublish_clearsActionColle Mockito.when(pluginExecutor.getHintMessages(Mockito.any(), Mockito.any())) .thenReturn(Mono.zip(Mono.just(new HashSet<>()), Mono.just(new HashSet<>()))); - ActionCollectionDTO actionCollectionDTO = new ActionCollectionDTO(); - actionCollectionDTO.setName("testCollection1"); - actionCollectionDTO.setPageId(testPage.getId()); - actionCollectionDTO.setApplicationId(testApp.getId()); - actionCollectionDTO.setWorkspaceId(workspaceId); - actionCollectionDTO.setPluginId(datasource.getPluginId()); - actionCollectionDTO.setVariables(List.of(new JSValue("test", "String", "test", true))); - actionCollectionDTO.setBody("collectionBody"); - actionCollectionDTO.setPluginType(PluginType.JS); - - // Create actions - ActionDTO action1 = new ActionDTO(); - action1.setName("testAction1"); - action1.setActionConfiguration(new ActionConfiguration()); - action1.getActionConfiguration().setBody("initial body"); - action1.getActionConfiguration().setIsValid(false); + ActionCollectionDTO actionCollectionDTO = + createActionCollection("testCollection1", "collectionBody", PluginType.JS); + ActionDTO action1 = createAction("testAction1", "initial body", false); actionCollectionDTO.setActions(List.of(action1)); // Create Js object @@ -901,17 +870,8 @@ public void testDeleteActionCollection_afterApplicationPublish_clearsActionColle String createdActionCollectionId = createdActionCollectionDTO.getId(); // Update JS object to create an action with same name as previously created action - ActionDTO action2 = new ActionDTO(); - action2.setName("testAction2"); - action2.setActionConfiguration(new ActionConfiguration()); - action2.getActionConfiguration().setBody("mockBody"); - action2.getActionConfiguration().setIsValid(false); - - ActionDTO action3 = new ActionDTO(); - action3.setName("testAction2"); - action3.setActionConfiguration(new ActionConfiguration()); - action3.getActionConfiguration().setBody("mockBody"); - action3.getActionConfiguration().setIsValid(false); + ActionDTO action2 = createAction("testAction2", "mockBody", false); + ActionDTO action3 = createAction("testAction2", "mockBody", false); actionCollectionDTO.setActions( List.of(createdActionCollectionDTO.getActions().get(0), action2, action3)); @@ -926,4 +886,85 @@ public void testDeleteActionCollection_afterApplicationPublish_clearsActionColle assertEquals(expectedMessage, error.getMessage()); }); } + + @Test + @WithUserDetails(value = "api_user") + public void testLayoutOnLoadActions_withTwoWidgetsAndSameBinding_callsCorrectActions() { + Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())).thenReturn(Mono.just(pluginExecutor)); + Mockito.when(pluginExecutor.getHintMessages(Mockito.any(), Mockito.any())) + .thenReturn(Mono.zip(Mono.just(new HashSet<>()), Mono.just(new HashSet<>()))); + + ActionCollectionDTO actionCollectionDTO = + createActionCollection("testCollection1", "collectionBody", PluginType.JS); + + // Create actions + ActionDTO action1 = createAction("myFunction", "return [{\"key\": \"value\"}];", true); + ActionDTO action2 = createAction("myFunction2", "mockBody", false); + + actionCollectionDTO.setActions(List.of(action1, action2)); + + // Create Layout with Table and Text Widgets + Layout layout = testPage.getLayouts().get(0); + layout.getDsl().put("widgetName", "MainContainer"); + ArrayList dslList = (ArrayList) layout.getDsl().get("children"); + JSONObject tableDsl = (JSONObject) dslList.get(0); + tableDsl.put("tableData", "{{testCollection1.myFunction.data}}"); + tableDsl.put("dynamicBindingPathList", createDynamicList("key", "tableData")); + tableDsl.put("dynamicPropertyPathList", createDynamicList("key", "tableData")); + + JSONObject textDsl = new JSONObject(); + textDsl.put("widgetName", "Text1"); + textDsl.put("type", "TEXT_WIDGET"); + textDsl.put("text", "{{testCollection1.myFunction.data}} + {{testCollection1.myFunction2.data}}"); + textDsl.put("dynamicBindingPathList", createDynamicList("key", "text")); + textDsl.put("dynamicPropertyPathList", createDynamicList("key", "text")); + + layout.setLayoutOnLoadActions(List.of()); + + dslList.add(textDsl); + + testPage.setLayouts(List.of(layout)); + + PageDTO updatedPage = + newPageService.updatePage(testPage.getId(), testPage).block(); + + // Create Js object + ActionCollectionDTO createdActionCollectionDTO = + layoutCollectionService.createCollection(actionCollectionDTO).block(); + assert createdActionCollectionDTO != null; + assert createdActionCollectionDTO.getId() != null; + String createdActionCollectionId = createdActionCollectionDTO.getId(); + + final Mono updatedActionCollectionDTOMono = + layoutCollectionService.updateUnpublishedActionCollection( + createdActionCollectionId, actionCollectionDTO); + + Mono pageWithMigratedDSLMono = + applicationPageService.getPageAndMigrateDslByBranchedPageId(testPage.getId(), false, false); + + StepVerifier.create(updatedActionCollectionDTOMono.zipWhen(actionCollectionDTO1 -> { + return pageWithMigratedDSLMono; + })) + .assertNext(tuple -> { + ActionCollectionDTO actionCollectionDTO1 = tuple.getT1(); + assertEquals(createdActionCollectionId, actionCollectionDTO1.getId()); + Mockito.verify(updateLayoutService, Mockito.times(2)) + .updatePageLayoutsByPageId(Mockito.anyString()); + actionCollectionDTO1 + .getActions() + .forEach(action -> assertNull(action.getErrorReports(), "Error reports should be null")); + + PageDTO pageWithMigratedDSL = tuple.getT2(); + List> layoutOnLoadActions = + pageWithMigratedDSL.getLayouts().get(0).getLayoutOnLoadActions(); + List> actualNames = layoutOnLoadActions.stream() + .map(set -> + set.stream().map(DslExecutableDTO::getName).collect(Collectors.toSet())) + .collect(Collectors.toList()); + List> expectedNames = + List.of(Set.of("testCollection1.myFunction", "testCollection1.myFunction2")); + assertEquals(expectedNames, actualNames, "layoutOnLoadActions should contain the expected names"); + }) + .verifyComplete(); + } } 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 4e48f49e0c87..afdc45a785b5 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 @@ -524,16 +524,16 @@ private Mono createComplexAppForExecuteOnLoad(Mono pageMono) "some dynamic {{\"anIgnoredAction.data:\" + aGetAction.data}}", "dynamicPost", """ - some dynamic {{ - (function(ignoredAction1){ - \tlet a = ignoredAction1.data - \tlet ignoredAction2 = { data: "nothing" } - \tlet b = ignoredAction2.data - \tlet c = "ignoredAction3.data" - \t// ignoredAction4.data - \treturn aPostAction.data - })(anotherPostAction.data)}} - """, + some dynamic {{ + (function(ignoredAction1){ + \tlet a = ignoredAction1.data + \tlet ignoredAction2 = { data: "nothing" } + \tlet b = ignoredAction2.data + \tlet c = "ignoredAction3.data" + \t// ignoredAction4.data + \treturn aPostAction.data + })(anotherPostAction.data)}} + """, "dynamicPostWithAutoExec", "some dynamic {{aPostActionWithAutoExec.data}}", "dynamicDelete", @@ -936,6 +936,69 @@ public void getActionsExecuteOnLoadWithAstLogic() { \t// ignoredAction4.data \treturn aPostAction.data })(anotherPostAction.data)"""; + + Mockito.when(astService.getPossibleReferencesFromDynamicBinding( + List.of( + " anotherDBAction.data.optional ", + "Collection.aSyncCollectionActionWithCall()", + "Collection.anAsyncCollectionActionWithCall()", + "Collection.aSyncCollectionActionWithoutCall.data", + "Collection.anAsyncCollectionActionWithoutCall.data", + "aPostActionWithAutoExec.data", + "aTableAction.data.child", + "\"anIgnoredAction.data:\" + aGetAction.data", + "aDBAction.data[0].irrelevant", + bindingValue), + EVALUATION_VERSION)) + .thenReturn(Flux.just( + Tuples.of( + " anotherDBAction.data.optional ", + new HashSet<>(Set.of("anotherDBAction.data.optional"))), + Tuples.of( + "Collection.aSyncCollectionActionWithCall()", + new HashSet<>(Set.of("Collection.aSyncCollectionActionWithCall"))), + Tuples.of( + "Collection.anAsyncCollectionActionWithCall()", + new HashSet<>(Set.of("Collection.anAsyncCollectionActionWithCall"))), + Tuples.of( + "Collection.aSyncCollectionActionWithoutCall.data", + new HashSet<>(Set.of("Collection.aSyncCollectionActionWithoutCall.data"))), + Tuples.of( + "Collection.anAsyncCollectionActionWithoutCall.data", + new HashSet<>(Set.of("Collection.anAsyncCollectionActionWithoutCall.data"))), + Tuples.of( + "aPostActionWithAutoExec.data", new HashSet<>(Set.of("aPostActionWithAutoExec.data"))), + Tuples.of("aTableAction.data.child", new HashSet<>(Set.of("aTableAction.data.child"))), + Tuples.of( + "\"anIgnoredAction.data:\" + aGetAction.data", + new HashSet<>(Set.of("aGetAction.data"))), + Tuples.of( + "aDBAction.data[0].irrelevant", new HashSet<>(Set.of("aDBAction.data[0].irrelevant"))), + Tuples.of(bindingValue, new HashSet<>(Set.of("aPostAction.data", "anotherPostAction.data"))))); + + Mockito.when(astService.getPossibleReferencesFromDynamicBinding( + List.of("aPostTertiaryAction.data", "aPostSecondaryAction.data"), EVALUATION_VERSION)) + .thenReturn(Flux.just( + Tuples.of("aPostTertiaryAction.data", new HashSet<>(Set.of("aPostTertiaryAction.data"))), + Tuples.of("aPostSecondaryAction.data", new HashSet<>(Set.of("aPostSecondaryAction.data"))))); + + Mockito.when(astService.getPossibleReferencesFromDynamicBinding( + List.of( + "aPostTertiaryAction.data", + "hiddenAction4.data", + "hiddenAction1.data", + "hiddenAction3.data", + "aPostSecondaryAction.data", + "hiddenAction2.data"), + EVALUATION_VERSION)) + .thenReturn(Flux.just( + Tuples.of("aPostTertiaryAction.data", new HashSet<>(Set.of("aPostTertiaryAction.data"))), + Tuples.of("hiddenAction4.data", new HashSet<>(Set.of("hiddenAction4.data"))), + Tuples.of("hiddenAction1.data", new HashSet<>(Set.of("hiddenAction1.data"))), + Tuples.of("hiddenAction3.data", new HashSet<>(Set.of("hiddenAction3.data"))), + Tuples.of("aPostSecondaryAction.data", new HashSet<>(Set.of("aPostSecondaryAction.data"))), + Tuples.of("hiddenAction2.data", new HashSet<>(Set.of("hiddenAction2.data"))))); + Mockito.when(astService.getPossibleReferencesFromDynamicBinding(List.of(bindingValue), EVALUATION_VERSION)) .thenReturn(Flux.just( Tuples.of(bindingValue, new HashSet<>(Set.of("aPostAction.data", "anotherPostAction.data")))));